Merge branch 'trunk' into fix-web-code-search-filename-extension-flags

This commit is contained in:
Sam Coe 2025-07-07 14:37:03 +02:00 committed by GitHub
commit d28a2bdf47
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
79 changed files with 2806 additions and 767 deletions

View file

@ -86,5 +86,5 @@ A member of the core team will [triage](../docs/triage.md) the design proposal.
[How to Contribute to Open Source]: https://opensource.guide/how-to-contribute/
[Using Pull Requests]: https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-requests
[GitHub Help]: https://docs.github.com/
[CLI Design System]: https://primer.style/cli/
[CLI Design System]: /docs/primer/
[Google Docs Template]: https://docs.google.com/document/d/1JIRErIUuJ6fTgabiFYfCH3x91pyHuytbfa0QLnTfXKM/edit#heading=h.or54sa47ylpg

View file

@ -7,7 +7,6 @@ updates:
ignore:
- dependency-name: "*"
update-types:
- version-update:semver-minor
- version-update:semver-major
- package-ecosystem: "github-actions"
directory: "/"

View file

@ -12,6 +12,17 @@ jobs:
- name: Checkout repository
uses: actions/checkout@v4
- name: Set up Go
uses: actions/setup-go@v5
with:
go-version-file: 'go.mod'
- name: Bump Go version
env:
GIT_COMMITTER_NAME: cli automation
GIT_AUTHOR_NAME: cli automation
GIT_COMMITTER_EMAIL: noreply@github.com
GIT_AUTHOR_EMAIL: noreply@github.com
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
bash .github/workflows/scripts/bump-go.sh --apply go.mod

View file

@ -76,6 +76,9 @@ if [[ "$CURRENT_TOOLCHAIN_DIRECTIVE" != "go$TOOLCHAIN_VERSION" ]]; then
echo " • toolchain $CURRENT_TOOLCHAIN_DIRECTIVE → go$TOOLCHAIN_VERSION"
fi
# ---- Run go mod tidy to ensure .0 minor version is added to go directive ----
go mod tidy
rm -f "$GO_MOD.bak"
git add "$GO_MOD"

View file

@ -0,0 +1,44 @@
# Setup environment variables used for testscript
env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
env2upper SECRET_NAME=${SCRIPT_NAME}_${RANDOM_STRING}
# Use gh as a credential helper
exec gh auth setup-git
# Create a repository with a file so it has a default branch
exec gh repo create ${ORG}/${REPO} --add-readme --private
# Defer repo cleanup
defer gh repo delete --yes ${ORG}/${REPO}
# Confirm organization secret does not exist, will fail admin:org scope missing
exec gh secret list --org ${ORG}
! stdout ${SECRET_NAME}
# Set an organization secret with no shared visibility, but no repos
exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --no-repos-selected
# Defer organization secret cleanup
defer gh secret delete ${SECRET_NAME} --org ${ORG}
# Verify new organization secret exists with shared visibility
exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME} --jq '.visibility'
stdout selected
# Verify the secret is not shared with any repositories
exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories | length'
stdout 0
# Set the same organization secret with shared visibility to the previously created repository
exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --repos ${REPO}
# Verify the secret is now shared with the repository
exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories[0].name'
stdout ${REPO}
# Set the same organization secret with shared visibility back to no repositories selected
exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --no-repos-selected
# Verify the secret is not shared with any repositories
exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories | length'
stdout 0

View file

@ -1,4 +1,6 @@
# Setup environment variables used for testscript
# This script will most likely fail because you are most likely targeting a repo that is not public and an org
# that is not on the right plan: https://docs.github.com/en/actions/how-tos/security-for-github-actions/security-guides/using-secrets-in-github-actions#creating-secrets-for-an-organization
env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
env2upper SECRET_NAME=${SCRIPT_NAME}_${RANDOM_STRING}

View file

@ -85,25 +85,15 @@ type PullRequest struct {
Assignees Assignees
AssignedActors AssignedActors
// AssignedActorsUsed is a GIGANTIC hack to carry around whether we expected AssignedActors to be requested
// on this PR. This is required because the Feature Detection of support for AssignedActors occurs inside the
// PR Finder, but knowledge of support is required at the command level. However, we can't easily construct
// the feature detector at the command level because it needs knowledge of the BaseRepo, which is only available
// inside the PR Finder. This is bad and we should feel bad.
//
// The right solution is to extract argument parsing from the PR Finder into each command, so that we have access
// to the BaseRepo and can construct the feature detector there. This is what happens in the issue commands with
// `shared.ParseIssueFromArg`.
AssignedActorsUsed bool
Labels Labels
ProjectCards ProjectCards
ProjectItems ProjectItems
Milestone *Milestone
Comments Comments
ReactionGroups ReactionGroups
Reviews PullRequestReviews
LatestReviews PullRequestReviews
ReviewRequests ReviewRequests
Labels Labels
ProjectCards ProjectCards
ProjectItems ProjectItems
Milestone *Milestone
Comments Comments
ReactionGroups ReactionGroups
Reviews PullRequestReviews
LatestReviews PullRequestReviews
ReviewRequests ReviewRequests
ClosingIssuesReferences ClosingIssuesReferences
}

View file

@ -14,7 +14,7 @@ our release schedule.
Install:
```bash
(type -p wget >/dev/null || (sudo apt update && sudo apt-get install wget -y)) \
(type -p wget >/dev/null || (sudo apt update && sudo apt install wget -y)) \
&& sudo mkdir -p -m 755 /etc/apt/keyrings \
&& out=$(mktemp) && wget -nv -O$out https://cli.github.com/packages/githubcli-archive-keyring.gpg \
&& cat $out | sudo tee /etc/apt/keyrings/githubcli-archive-keyring.gpg > /dev/null \

15
docs/primer/README.md Normal file
View file

@ -0,0 +1,15 @@
# GitHub CLI Primer Design
These guidelines are a collection of principles, foundations and usage guidelines for designing GitHub command line products.
## [Components](components)
Design guidance on how we format content in in the Terminal through text formatting, color and font weights.
## [Foundations](foundations)
Design concepts and constraints that can help create a better Terminal like experience for GitHub.
## [Getting started](getting-started)
Primer is also a design system for Terminal like implementations of GitHub. If youre just starting out with creating those kind of experiences, heres a list of principles and design foundations to get you started.

View file

@ -0,0 +1,234 @@
# Components
Components are consistent, reusable patterns that we use throughout the command line tool.
## Syntax
We show meaning or objects through syntax such as angled brackets, square brackets, curly brackets, parenthesis, and color.
### Branches
Display branch names in brackets and/or cyan
![A branch name in brackets and cyan](images/Syntax-Branch.png)
### Labels
Display labels in parenthesis and/or gray
![A label name in parenthesis and gray](images/Syntax-Label.png)
### Repository
Display repository names in bold where appropriate
![A repository name in bold](images/Syntax-Repo.png)
### Help
Use consistent syntax in [help pages](/docs/command-line-syntax.md) to explain command usage.
#### Literal text
Use plain text for parts of the command that cannot be changed
```shell
gh help
```
The argument help is required in this command.
#### Placeholder values
Use angled brackets to represent a value the user must replace. No other expressions can be contained within the angled brackets.
```shell
gh pr view <issue-number>
```
Replace "issue-number" with an issue number.
#### Optional arguments
Place optional arguments in square brackets. Mutually exclusive arguments can be included inside square brackets if they are separated with vertical bars.
```shell
gh pr checkout [--web]
```
The argument `--web` is optional.
```shell
gh pr view [<number> | <url>]
```
The "number" and "url" arguments are optional.
#### Required mutually exclusive arguments
Place required mutually exclusive arguments inside braces, separate arguments with vertical bars.
```shell
gh pr {view | create}
```
#### Repeatable arguments
Ellipsis represent arguments that can appear multiple times
```shell
gh pr close <pr-number>...
```
#### Variable naming
For multi-word variables use dash-case (all lower case with words separated by dashes)
```shell
gh pr checkout <issue-number>
```
#### Additional examples
Optional argument with placeholder:
```shell
<command> <subcommand> [<arg>]
```
Required argument with mutually exclusive options:
```shell
<command> <subcommand> {<path> | <string> | literal}
```
Optional argument with mutually exclusive options:
```shell
<command> <subcommand> [<path> | <string>]
```
## Prompts
Generally speaking, prompts are the CLIs version of forms.
- Use prompts for entering information
- Use a prompt when user intent is unclear
- Make sure to provide flags for all prompts
### Yes/No
Use for yes/no questions, usually a confirmation. The default (what will happen if you enter nothing and hit enter) is in caps.
![An example of a yes/no prompt](images/Prompt-YesNo.png)
### Short text
Use to enter short strings of text. Enter will accept the auto fill if available
![An example of a short text prompt](images/Prompt-ShortText.png)
### Long text
Use to enter large bodies of text. E key will open the users preferred editor, and Enter will skip.
![An example of a long text prompt](images/Prompt-LongText.png)
### Radio select
Use to select one option
![An example of a radio select prompt](images/Prompt-RadioSelect.png)
### Multi select
Use to select multiple options
![An example of a multi select prompt](images/Prompt-MultiSelect.png)
## State
The CLI reflects how GitHub.com displays state through [color](/docs/primer/foundations#color) and [iconography](/docs/primer/foundations#iconography).
![A collection of examples of state from various command outputs](images/States.png)
## Progress indicators
For processes that might take a while, include a progress indicator with context on whats happening.
![An example of a loading spinner when forking a repository](images/Progress-Spinner.png)
## Headers
When viewing output that could be unclear, headers can quickly set context for what the user is seeing and where they are.
### Examples
![An example of the header of the `gh pr create` command](images/Headers-Examples.png)
The header of the `gh pr create` command reassures the user that they're creating the correct pull request.
![An example of the header of the `gh pr list` command](images/Headers-gh-pr-list.png)
The header of the `gh pr list` command sets context for what list the user is seeing.
## Lists
Lists use tables to show information.
- State is shown in color.
- A header is used for context.
- Information shown may be branch names, dates, or what is most relevant in context.
![An example of gh pr list](images/Lists-gh-pr-list.png)
## Detail views
Single item views show more detail than list views. The body of the item is rendered indented. The items URL is shown at the bottom.
![An example of gh issue view](images/Detail-gh-issue-view.png)
## Empty states
Make sure to include empty messages in command outputs when appropriate.
![The empty state of the gh pr status command](images/Empty-states-1.png)
The empty state of `gh pr status`
![The empty state of the gh issue list command](images/Empty-states-2.png)
The empty state of `gh issue list`
## Help pages
Help commands can exist at any level:
- Top level (`gh`)
- Second level (`gh [command]`)
- Third level (`gh [command] [subcommand]`)
Each can be accessed using the `--help` flag, or using `gh help [command]`.
Each help page includes a combination of different sections.
### Required sections
- Usage
- Core commands
- Flags
- Learn more
- Inherited flags
### Other available sections
- Additional commands
- Examples
- Arguments
- Feedback
### Example
![The output of gh help](images/Help.png)

Binary file not shown.

After

Width:  |  Height:  |  Size: 59 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 40 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 11 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 16 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 44 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 223 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 44 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 6.8 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 7.9 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 24 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 16 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 4.5 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 4.1 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 6.8 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 60 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 2.9 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.1 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.7 KiB

View file

@ -0,0 +1,214 @@
# Foundations
Design concepts and constraints that can help create a better Terminal like experience for GitHub.
## Language
Language is the most important tool at our disposal for creating a clear, understandable product. Having clear language helps us create memorable commands that are clear in what they will do.
We generally follow this structure:
| **gh** | **`<command>`** | **`<subcommand>`** | **[value]** | **[flags]** | **[value]** |
| --- | ----------- | -------------- | ------- | --------- | ------- |
| gh | issue | view | 234 | --web | - |
| gh | pr | create | - | --title | “Title” |
| gh | repo | fork | cli/cli | --clone | false |
| gh | pr | status | - | - | - |
| gh | issue | list | - | --state | closed |
| gh | pr | review | 234 | --approve | - |
**Command:** The object you want to interact with
**Subcommand:** The action you want to take on that object. Most `gh` commands contain a command and subcommand. These may take arguments, such as issue/PR numbers, URLs, file names, OWNER/REPO, etc.
**Flag:** A way to modify the command, also may be called “options”. You can use multiple flags. Flags can take values, but dont always. Flags always have a long version with two dashes `(--state)` but often also have a shortcut with one dash and one letter `(-s)`. Its possible to chain shorthand flags: `-sfv` is the same as `-s -f -v`
**Values:** Are passed to the commands or flags
- The most common command values are:
- Issue or PR number
- The “owner/repo” pair
- URLs
- Branch names
- File names
- The possible flag values depend on the flag:
- `--state` takes `{closed | open | merged}`
- `--clone` is a boolean flag
- `--title` takes a string
- `--limit` takes an integer
_Tip: To get a better sense of what feels right, try writing out the commands in the CLI a few different ways._
<table>
<tr>
<td>
Do: Use a flag for modifiers of actions.
<img alt="`gh pr review --approve` command" src="images/Language-06.png" />
</td>
<td>
Don't: Avoid making modifiers their own commands.
<img alt="`gh pr approve` command" src="images/Language-03.png" />
</td>
</tr>
</table>
**When designing your commands language system:**
- Use [GitHub language](/getting-started/principles#make-it-feel-like-github)
- Use unambiguous language that cant be confused for something else
- Use shorter phrases if possible and appropriate
<table>
<tr>
<td>
Do: Use language that can't be misconstrued.
<img alt="`gh pr create` command" src="images/Language-05.png" />
</td>
<td>
Don't: Avoid language that can be interpreted in multiple ways ("open in browser" or "open a pull request" here).
<img alt="`gh pr open` command" src="images/Language-02.png" />
</td>
</tr>
</table>
<table>
<tr>
<td>
Do: Use understood shorthands to save characters to type.
<img alt="`gh repo view` command" src="images/Language-04.png" />
</td>
<td>
Don't: Avoid long words in commands if there's a reasonable alternative.
<img alt="`gh repository view` command" src="images/Language-01.png" />
</td>
</tr>
</table>
## Typography
Everything in a command line interface is text, so type hierarchy is important. All type is the same size and font, but you can still create type hierarchy using font weight and space.
![An example of normal weight, and bold weight. Italics is striked through since it's not used.](images/Typography.png)
- People customize their fonts, but you can assume it will be a monospace
- Monospace fonts inherently create visual order
- Fonts may have variable unicode support
### Accessibility
If you want to ensure that a screen reader will read a pause, you can use a:
- period (`.`)
- comma (`,`)
- colon (`:`)
## Spacing
You can use the following to create hierarchy and visual rhythm:
- Line breaks
- Tables
- Indentation
Do: Use space to create more legible output.
<img alt="`gh pr status` command indenting content under sections" src="images/Spacing-gh-pr-status.png" />
Don't: Not using space makes output difficult to parse.
<img alt="`gh pr status` command where content is not indented, making it harder to read" src="images/Spacing-gh-pr-status-compressed.png" />
## Color
Terminals reliably recognize the 8 basic ANSI colors. There are also bright versions of each of these colors that you can use, but less reliably.
<img alt="A table describing the usage of the 8 basic colors." src="images/Colors.png" />
### Things to note
- Background color is available but we havent taken advantage of it yet.
- Some terminals do not reliably support 256-color escape sequences.
- Users can customize how their terminal displays the 8 basic colors, but thats opt-in (for example, the user knows theyre making their greens not green).
- Only use color to [enhance meaning](https://primer.style/design/accessibility/guidelines#use-of-color), not to communicate meaning.
## Iconography
Since graphical image support in terminal emulators is unreliable, we rely on Unicode for iconography. When applying iconography consider:
- People use different fonts that will have varying Unicode support
- Only use iconography to [enhance meaning](https://primer.style/design/global/accessibility#visual-accessibility), not to communicate meaning
_Note: In Windows, Powershells default font (Lucida Console) has poor Unicode support. Microsoft suggests changing it for more Unicode support._
**Symbols currently used:**
```
✓ Success
- Neutral
✗ Failure
+ Changes requested
! Alert
```
<table>
<tr>
<td>
Do: Use checks for success messages.
<img alt="✓ Checks passing" src="images/Iconography-1.png" />
</td>
<td>
Don't: Don't use checks for failure messages.
<img alt="✓ Checks failing" src="images/Iconography-2.png" />
</td>
</tr>
</table>
<table>
<tr>
<td>
Do: Use checks for success of closing or deleting.
<img alt="✓ Issue closed" src="images/Iconography-3.png" />
</td>
<td>
Do: Don't use alerts when closing or deleting.
<img alt="! Issue closed" src="images/Iconography-4.png" />
</td>
</tr>
</table>
## Scriptability
Make choices that ensure that creating automations or scripts with GitHub commands is obvious and frictionless. Practically, this means:
- Create flags for anything interactive
- Ensure flags have clear language and defaults
- Consider what should be different for terminal vs machine output
### In terminal
![An example of gh pr list](images/Scriptability-gh-pr-list.png)
### Through pipe
![An example of gh pr list piped through the cat command](images/Scriptability-gh-pr-list-machine.png)
### Differences to note in machine output
- No color or styling
- State is explicitly written, not implied from color
- Tabs between columns instead of table layout, since `cut` uses tabs as a delimiter
- No truncation
- Exact date format
- No header
## Customizability
Be aware that people exist in different environments and may customize their setups. Customizations include:
- **Shell:** shell prompt, shell aliases, PATH and other environment variables, tab-completion behavior
- **Terminal:** font, color scheme, and keyboard shortcuts
- **Operating system**: language input options, accessibility settings
The CLI tool itself is also customizable. These are all tools at your disposal when designing new commands.
- Aliasing: [`gh alias set`](https://cli.github.com/manual/gh_alias_set)
- Preferences: [`gh config set`](https://cli.github.com/manual/gh_config_set)
- Environment variables: `NO_COLOR`, `EDITOR`, etc

Binary file not shown.

After

Width:  |  Height:  |  Size: 35 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.8 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.5 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.3 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.3 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 4.2 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 2.7 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.3 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.2 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 4.2 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 49 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 44 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 60 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 62 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 12 KiB

View file

@ -0,0 +1,131 @@
# Getting Started
## Principles
### Reasonable defaults, easy overrides
Optimize for what most people will need to do most of the time, but make it easy for people to adjust it to their needs. Often this means considering the default behavior of each command, and how it might need to be adjusted with flags.
### Make it feel like GitHub
Using this tool, it should be obvious that its GitHub and not anything else. Use details that are specific to GitHub, such as language or color. When designing output, reflect the GitHub.com interface as much as possible and appropriate.
<table>
<tr>
<td>
Do: Use language accurate to GitHub.com.
<img alt="`gh pr close` command" src="images/Principle2-05.png" />
</td>
<td>
Don't: Don't use language that GitHub.com doesn't use.
<img alt="`gh pr delete` command" src="images/Principle2-02.png" />
</td>
</tr>
</table>
<table>
<tr>
<td>
Do: Use sentence case.
<img alt="Pull request with request being a lowercase r" src="images/Principle2-04.png" />
</td>
<td>
Don't: Don't use title case.
<img alt="Pull Request with Request being an uppercase R" src="images/Principle2-01.png" />
</td>
</tr>
</table>
**Resources**
- [GitHub Brand Content Guide](https://brand.github.com)
### Reduce cognitive load
Command line interfaces are not as visually intuitive as graphical interfaces. They have very few affordances (indicators of use), rely on memory, and are often unforgiving of mistakes. We do our best to design our commands to mitigate this.
Reducing cognitive load is necessary for [making an accessible product](https://www.w3.org/TR/coga-usable/#summary) .
**Ways to reduce cognitive load**
- Include confirm steps, especially for riskier commands
- Include headers to help set context for output
- Ensure consistent command language to make memorizing easier
- Ensure similar commands are visually and behaviorally parallel. \* For example, any create command should behave the same
- Anticipate what people might want to do next. \* For example, we ask if you want to delete your branch after you merge.
- Anticipate what mistakes people might make
### Bias towards terminal, but make it easy to get to the browser
We want to help people stay in the terminal wherever they might want to maintain focus and reduce context switching, but when its necessary to jump to GitHub.com make it obvious, fast, and easy. Certain actions are probably better to do in a visual interface.
![A prompt asking 'What's next?' with the choice 'Preview in browser' selected.](images/Principle4-01.png)
A preview in browser step helps users create issues and pull requests more smoothly.
![The `gh pr create command` with `--title` and `--body` flags outputting a pull request URL.](images/Principle4-02.png)
Many commands output the relevant URL at the end.
![The `gh issue view` command with the `--web` flag. The output is opening a URL in the browser.](images/Principle4-03.png)
Web flags help users jump to the browser quickly
## Process
When designing for the command line, consider:
### 1. What the command does
- What makes sense to do from a terminal? What doesnt?
- What might people want to automate?
- What is the default behavior? What flags might you need to change that behavior?
- What might people try and fail to do and how can you anticipate that?
### 2. What the command is called
- What should the [command language system](/docs/primer/foundations#language) be?
- What should be a command vs a flag?
- How can you align the language of the new command with the existing commands?
### 3. What the command outputs
- What can you do to make the CLI version [feel like the GitHub.com version](#make-it-feel-like-github), using [color](/docs/primer/foundations#color), [language](/docs/primer/foundations#language), [spacing](/docs/primer/foundations#spacing), info shown, etc?
- How should the [machine output](/docs/primer/foundations#scriptability) differ from the interactive behavior?
### 4. How you explain your command
- You will need to provide a short and long description of the command for the [help pages](/docs/primer/components#help).
### 5. How people discover your command
- Are there ways to integrate CLI into the feature where it exists on other platforms?
## Prototyping
When designing for GitHub CLI, there are several ways you can go about prototyping your ideas.
### Google Docs
![A screenshot of the Google Docs template](images/Prototyping-GoogleDocs.png)
Best for simple quick illustrations of most ideas
Use [this template](https://docs.google.com/document/d/1JIRErIUuJ6fTgabiFYfCH3x91pyHuytbfa0QLnTfXKM/edit?usp=sharing), or format your document with these steps:
1. Choose a dark background (File > Page Setup > Page Color)
1. Choose a light text color
1. Choose a monospace font
**Tips**
- Mix it up since peoples setups change so much. Not everyone uses dark background!
- Make use of the document outline and headers to help communicate your ideas
### Figma
![A screenshot of the Figma library](images/Prototyping-Figma.png)
If you need to show a process unfolding over time, or need to show a prototype that feels more real to users, Figma or code prototypes are best.
[**Figma library**](https://www.figma.com/file/zYsBk5KFoMlovE4g2f4Wkg/Primer-Command-Line) (accessible to GitHub staff only)

Binary file not shown.

After

Width:  |  Height:  |  Size: 3 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 3 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 14 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 2.8 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 2.9 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 17 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 13 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 15 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 169 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 85 KiB

2
go.mod
View file

@ -39,7 +39,7 @@ require (
github.com/mattn/go-colorable v0.1.14
github.com/mattn/go-isatty v0.0.20
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
github.com/microsoft/dev-tunnels v0.0.25
github.com/microsoft/dev-tunnels v0.1.13
github.com/muhammadmuzzammil1998/jsonc v1.0.0
github.com/opentracing/opentracing-go v1.2.0
github.com/rivo/tview v0.0.0-20250625164341-a4a78f1e05cb

4
go.sum
View file

@ -370,8 +370,8 @@ github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQ
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwXFM08ygZfk=
github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA=
github.com/microsoft/dev-tunnels v0.0.25 h1:UlMKUI+2O8cSu4RlB52ioSyn1LthYSVkJA+CSTsdKoA=
github.com/microsoft/dev-tunnels v0.0.25/go.mod h1:frU++12T/oqxckXkDpTuYa427ncguEOodSPZcGCCrzQ=
github.com/microsoft/dev-tunnels v0.1.13 h1:bp1qqCvP/5iLol1Vz0c/lM2sexG7Gd8fRGcGv58vZdE=
github.com/microsoft/dev-tunnels v0.1.13/go.mod h1:Jvr6RlyjUXomM6KsDmIQbq+hhKd5mWrBcv3MEsa78dc=
github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw=
github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s=
github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=

View file

@ -132,7 +132,9 @@ func getTunnelManager(tunnelProperties api.TunnelProperties, httpClient *http.Cl
}
// Create the tunnel manager
tunnelManager, err = tunnels.NewManager(userAgent, nil, url, httpClient)
// This api version seems to be the only acceptable api version: https://github.com/microsoft/dev-tunnels/blob/bf96ae5a128041d1a23f81d53a47e9e6c26fdc8d/go/tunnels/manager.go#L66
apiVersion := "2023-09-27-preview"
tunnelManager, err = tunnels.NewManager(userAgent, nil, url, httpClient, apiVersion)
if err != nil {
return nil, fmt.Errorf("error creating tunnel manager: %w", err)
}

View file

@ -14,6 +14,7 @@ import (
"net/http/httptest"
"net/url"
"regexp"
"strconv"
"strings"
"sync"
"time"
@ -25,7 +26,28 @@ import (
"golang.org/x/crypto/ssh"
)
func NewMockHttpClient() (*http.Client, error) {
type mockClientOpts struct {
ports map[int]tunnels.TunnelPort // Port number to protocol
}
type mockClientOpt func(*mockClientOpts)
// WithSpecificPorts allows you to specify a map of ports to TunnelPorts that will be returned by the mock HTTP client.
// Note that this does not take a copy of the map, so you should not modify the map after passing it to this function.
func WithSpecificPorts(ports map[int]tunnels.TunnelPort) mockClientOpt {
return func(opts *mockClientOpts) {
opts.ports = ports
}
}
func NewMockHttpClient(opts ...mockClientOpt) (*http.Client, error) {
mockClientOpts := &mockClientOpts{}
for _, opt := range opts {
opt(mockClientOpts)
}
specifiedPorts := mockClientOpts.ports
accessToken := "tunnel access-token"
relayServer, err := newMockrelayServer(withAccessToken(accessToken))
if err != nil {
@ -35,7 +57,7 @@ func NewMockHttpClient() (*http.Client, error) {
hostURL := strings.Replace(relayServer.URL(), "http://", "ws://", 1)
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var response []byte
if r.URL.Path == "/api/v1/tunnels/tunnel-id" {
if r.URL.Path == "/tunnels/tunnel-id" {
tunnel := &tunnels.Tunnel{
AccessTokens: map[tunnels.TunnelAccessScope]string{
tunnels.TunnelAccessScopeConnect: accessToken,
@ -54,54 +76,141 @@ func NewMockHttpClient() (*http.Client, error) {
if err != nil {
log.Fatalf("json.Marshal returned an error: %v", err)
}
} else if strings.HasPrefix(r.URL.Path, "/api/v1/tunnels/tunnel-id/ports") {
// Use regex to check if the path ends with a number
match, err := regexp.MatchString(`\/\d+$`, r.URL.Path)
if err != nil {
log.Fatalf("regexp.MatchString returned an error: %v", err)
}
// If the path ends with a number, it's a request for a specific port
if match || r.Method == http.MethodPost {
_, _ = w.Write(response)
return
} else if strings.HasPrefix(r.URL.Path, "/tunnels/tunnel-id/ports") {
// Use regex to capture the port number from the end of the path
re := regexp.MustCompile(`\/(\d+)$`)
matches := re.FindStringSubmatch(r.URL.Path)
targetingSpecificPort := len(matches) > 0
if targetingSpecificPort {
if r.Method == http.MethodDelete {
w.WriteHeader(http.StatusOK)
return
}
tunnelPort := &tunnels.TunnelPort{
if r.Method == http.MethodGet {
// If no ports were configured, then we assume that every request for a port is valid.
if specifiedPorts == nil {
response, err := json.Marshal(tunnels.TunnelPort{
AccessControl: &tunnels.TunnelAccessControl{
Entries: []tunnels.TunnelAccessControlEntry{},
},
})
if err != nil {
log.Fatalf("json.Marshal returned an error: %v", err)
}
_, _ = w.Write(response)
return
} else {
// Otherwise we'll fetch the port from our configured ports and include the protocol in the response.
port, err := strconv.Atoi(matches[1])
if err != nil {
log.Fatalf("strconv.Atoi returned an error: %v", err)
}
tunnelPort, ok := specifiedPorts[port]
if !ok {
w.WriteHeader(http.StatusNotFound)
return
}
response, err := json.Marshal(tunnelPort)
if err != nil {
log.Fatalf("json.Marshal returned an error: %v", err)
}
_, _ = w.Write(response)
return
}
}
// Else this is an unexpected request, fall through to 404 at the bottom
}
// If it's a PUT request, we assume it's for creating a new port so we'll do some validation
// and then return a stub.
if r.Method == http.MethodPut {
// If a port was already configured with this number, and the protocol has changed, return a 400 Bad Request.
if specifiedPorts != nil {
port, err := strconv.Atoi(matches[1])
if err != nil {
log.Fatalf("strconv.Atoi returned an error: %v", err)
}
var portRequest tunnels.TunnelPort
if err := json.NewDecoder(r.Body).Decode(&portRequest); err != nil {
log.Fatalf("json.NewDecoder returned an error: %v", err)
}
tunnelPort, ok := specifiedPorts[port]
if ok {
if tunnelPort.Protocol != portRequest.Protocol {
w.WriteHeader(http.StatusBadRequest)
return
}
}
// Create or update the new port entry.
specifiedPorts[port] = portRequest
}
response, err := json.Marshal(tunnels.TunnelPort{
AccessControl: &tunnels.TunnelAccessControl{
Entries: []tunnels.TunnelAccessControlEntry{},
},
}
})
// Convert the tunnel to JSON and write it to the response
response, err = json.Marshal(*tunnelPort)
if err != nil {
log.Fatalf("json.Marshal returned an error: %v", err)
}
} else {
// If the path doesn't end with a number and we aren't making a POST request, return an array of ports
tunnelPorts := []tunnels.TunnelPort{
{
AccessControl: &tunnels.TunnelAccessControl{
Entries: []tunnels.TunnelAccessControlEntry{},
},
},
}
response, err = json.Marshal(tunnelPorts)
if err != nil {
log.Fatalf("json.Marshal returned an error: %v", err)
}
_, _ = w.Write(response)
return
}
// Finally, if it's not targeting a specific port or a POST request, we return a list of ports, either
// totally stubbed, or whatever was configured in the mock client options.
if specifiedPorts == nil {
response, err := json.Marshal(tunnels.TunnelPortListResponse{
Value: []tunnels.TunnelPort{
{
AccessControl: &tunnels.TunnelAccessControl{
Entries: []tunnels.TunnelAccessControlEntry{},
},
},
},
})
if err != nil {
log.Fatalf("json.Marshal returned an error: %v", err)
}
_, _ = w.Write(response)
return
} else {
var ports []tunnels.TunnelPort
for _, tunnelPort := range specifiedPorts {
ports = append(ports, tunnelPort)
}
response, err := json.Marshal(tunnels.TunnelPortListResponse{
Value: ports,
})
if err != nil {
log.Fatalf("json.Marshal returned an error: %v", err)
}
_, _ = w.Write(response)
return
}
} else {
w.WriteHeader(http.StatusNotFound)
return
}
// Write the response
_, _ = w.Write(response)
}))
url, err := url.Parse(mockServer.URL)

View file

@ -12,9 +12,9 @@ import (
)
const (
githubSubjectId = "1"
InternalPortTag = "InternalPort"
UserForwardedPortTag = "UserForwardedPort"
githubSubjectId = "1"
InternalPortLabel = "InternalPort"
UserForwardedPortLabel = "UserForwardedPort"
)
const (
@ -108,7 +108,26 @@ func (fwd *CodespacesPortForwarder) ForwardPort(ctx context.Context, opts Forwar
return fmt.Errorf("error converting port: %w", err)
}
tunnelPort := tunnels.NewTunnelPort(port, "", "", tunnels.TunnelProtocolHttp)
// In v0.0.25 of dev-tunnels, the dev-tunnel manager `CreateTunnelPort` would "accept" requests that
// change the port protocol but they would not result in any actual change. This has changed, resulting in
// an error `Invalid arguments. The tunnel port protocol cannot be changed.`. It's not clear why the previous
// behaviour existed, whether it was truly the API version, or whether the `If-Not-Match` header being set inside
// `CreateTunnelPort` avoided the server accepting the request to change the protocol and that has since regressed.
//
// In any case, now we check whether a port exists with the given port number, if it does, we use the existing protocol.
// If it doesn't exist, we default to HTTP, which was the previous behaviour for all ports.
protocol := tunnels.TunnelProtocolHttp
existingPort, err := fwd.connection.TunnelManager.GetTunnelPort(ctx, fwd.connection.Tunnel, opts.Port, fwd.connection.Options)
if err != nil && !strings.Contains(err.Error(), "404") {
return fmt.Errorf("error checking whether tunnel port already exists: %v", err)
}
if existingPort != nil {
protocol = tunnels.TunnelProtocol(existingPort.Protocol)
}
tunnelPort := tunnels.NewTunnelPort(port, "", "", protocol)
// If no visibility is provided, Dev Tunnels will use the default (private)
if opts.Visibility != "" {
@ -136,9 +155,9 @@ func (fwd *CodespacesPortForwarder) ForwardPort(ctx context.Context, opts Forwar
// Tag the port as internal or user forwarded so we know if it needs to be shown in the UI
if opts.Internal {
tunnelPort.Tags = []string{InternalPortTag}
tunnelPort.Labels = []string{InternalPortLabel}
} else {
tunnelPort.Tags = []string{UserForwardedPortTag}
tunnelPort.Labels = []string{UserForwardedPortLabel}
}
// Create the tunnel port
@ -362,8 +381,8 @@ func visibilityToAccessControlEntries(visibility string) []tunnels.TunnelAccessC
// IsInternalPort returns true if the port is internal.
func IsInternalPort(port *tunnels.TunnelPort) bool {
for _, tag := range port.Tags {
if strings.EqualFold(tag, InternalPortTag) {
for _, label := range port.Labels {
if strings.EqualFold(label, InternalPortLabel) {
return true
}
}

View file

@ -105,10 +105,10 @@ func TestAccessControlEntriesToVisibility(t *testing.T) {
func TestIsInternalPort(t *testing.T) {
internalPort := &tunnels.TunnelPort{
Tags: []string{"InternalPort"},
Labels: []string{"InternalPort"},
}
userForwardedPort := &tunnels.TunnelPort{
Tags: []string{"UserForwardedPort"},
Labels: []string{"UserForwardedPort"},
}
tests := []struct {
@ -137,3 +137,131 @@ func TestIsInternalPort(t *testing.T) {
})
}
}
func TestForwardPortDefaultsToHTTPProtocol(t *testing.T) {
codespace := &api.Codespace{
Name: "codespace-name",
State: api.CodespaceStateAvailable,
Connection: api.CodespaceConnection{
TunnelProperties: api.TunnelProperties{
ConnectAccessToken: "tunnel access-token",
ManagePortsAccessToken: "manage-ports-token",
ServiceUri: "http://global.rel.tunnels.api.visualstudio.com/",
TunnelId: "tunnel-id",
ClusterId: "usw2",
Domain: "domain.com",
},
},
RuntimeConstraints: api.RuntimeConstraints{
AllowedPortPrivacySettings: []string{"public", "private"},
},
}
// Given there are no forwarded ports.
tunnelPorts := map[int]tunnels.TunnelPort{}
httpClient, err := connection.NewMockHttpClient(
connection.WithSpecificPorts(tunnelPorts),
)
if err != nil {
t.Fatalf("NewMockHttpClient returned an error: %v", err)
}
connection, err := connection.NewCodespaceConnection(t.Context(), codespace, httpClient)
if err != nil {
t.Fatalf("NewCodespaceConnection returned an error: %v", err)
}
fwd, err := NewPortForwarder(t.Context(), connection)
if err != nil {
t.Fatalf("NewPortForwarder returned an error: %v", err)
}
// When we forward a port without an existing one to use for a protocol, it should default to HTTP.
if err := fwd.ForwardPort(t.Context(), ForwardPortOpts{
Port: 1337,
}); err != nil {
t.Fatalf("ForwardPort returned an error: %v", err)
}
ports, err := fwd.ListPorts(t.Context())
if err != nil {
t.Fatalf("ListPorts returned an error: %v", err)
}
if len(ports) != 1 {
t.Fatalf("expected 1 port, got %d", len(ports))
}
if ports[0].Protocol != string(tunnels.TunnelProtocolHttp) {
t.Fatalf("expected port protocol to be http, got %s", ports[0].Protocol)
}
}
func TestForwardPortRespectsProtocolOfExistingTunneledPorts(t *testing.T) {
codespace := &api.Codespace{
Name: "codespace-name",
State: api.CodespaceStateAvailable,
Connection: api.CodespaceConnection{
TunnelProperties: api.TunnelProperties{
ConnectAccessToken: "tunnel access-token",
ManagePortsAccessToken: "manage-ports-token",
ServiceUri: "http://global.rel.tunnels.api.visualstudio.com/",
TunnelId: "tunnel-id",
ClusterId: "usw2",
Domain: "domain.com",
},
},
RuntimeConstraints: api.RuntimeConstraints{
AllowedPortPrivacySettings: []string{"public", "private"},
},
}
// Given we already have a port forwarded with an HTTPS protocol.
tunnelPorts := map[int]tunnels.TunnelPort{
1337: {
Protocol: string(tunnels.TunnelProtocolHttps),
AccessControl: &tunnels.TunnelAccessControl{
Entries: []tunnels.TunnelAccessControlEntry{},
},
},
}
httpClient, err := connection.NewMockHttpClient(
connection.WithSpecificPorts(tunnelPorts),
)
if err != nil {
t.Fatalf("NewMockHttpClient returned an error: %v", err)
}
connection, err := connection.NewCodespaceConnection(t.Context(), codespace, httpClient)
if err != nil {
t.Fatalf("NewCodespaceConnection returned an error: %v", err)
}
fwd, err := NewPortForwarder(t.Context(), connection)
if err != nil {
t.Fatalf("NewPortForwarder returned an error: %v", err)
}
// When we forward a port, it would typically default to HTTP, to which the mock server would respond with a 400,
// but it should respect the existing port's protocol and forward it as HTTPS.
if err := fwd.ForwardPort(t.Context(), ForwardPortOpts{
Port: 1337,
}); err != nil {
t.Fatalf("ForwardPort returned an error: %v", err)
}
ports, err := fwd.ListPorts(t.Context())
if err != nil {
t.Fatalf("ListPorts returned an error: %v", err)
}
if len(ports) != 1 {
t.Fatalf("expected 1 port, got %d", len(ports))
}
if ports[0].Protocol != string(tunnels.TunnelProtocolHttps) {
t.Fatalf("expected port protocol to be https, got %s", ports[0].Protocol)
}
}

View file

@ -263,12 +263,12 @@ func TestNewCmdEdit(t *testing.T) {
},
{
name: "argument is a URL",
input: "https://github.com/cli/cli/issues/23",
input: "https://example.com/cli/cli/issues/23",
output: EditOptions{
IssueNumbers: []int{23},
Interactive: true,
},
expectedBaseRepo: ghrepo.New("cli", "cli"),
expectedBaseRepo: ghrepo.NewWithHost("cli", "cli", "example.com"),
wantsErr: false,
},
{

View file

@ -3,6 +3,7 @@ package edit
import (
"fmt"
"net/http"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
@ -81,12 +82,28 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.Finder = shared.NewFinder(f)
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
if len(args) > 0 {
opts.SelectorArg = args[0]
}
if opts.SelectorArg != "" {
// If a URL is provided, we need to parse it to override the
// base repository, especially the hostname part. That's because
// we need a feature detector down in this command, and that
// needs to know the API host. If the command is run outside of
// a git repo, we cannot instantiate the detector unless we have
// already parsed the URL.
if baseRepo, _, err := shared.ParseURL(opts.SelectorArg); err == nil {
opts.BaseRepo = func() (ghrepo.Interface, error) {
return baseRepo, nil
}
}
}
flags := cmd.Flags()
bodyProvided := flags.Changed("body")
@ -203,17 +220,38 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
}
func editRun(opts *EditOptions) error {
findOptions := shared.FindOptions{
Selector: opts.SelectorArg,
Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "labels", "projectCards", "projectItems", "milestone", "assignees"},
Detector: opts.Detector,
}
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
if opts.Detector == nil {
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost())
}
findOptions := shared.FindOptions{
Selector: opts.SelectorArg,
Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "labels", "projectCards", "projectItems", "milestone"},
Detector: opts.Detector,
}
issueFeatures, err := opts.Detector.IssueFeatures()
if err != nil {
return err
}
if issueFeatures.ActorIsAssignable {
findOptions.Fields = append(findOptions.Fields, "assignedActors")
} else {
findOptions.Fields = append(findOptions.Fields, "assignees")
}
pr, repo, err := opts.Finder.Find(findOptions)
if err != nil {
return err
@ -225,7 +263,7 @@ func editRun(opts *EditOptions) error {
editable.Body.Default = pr.Body
editable.Base.Default = pr.BaseRefName
editable.Reviewers.Default = pr.ReviewRequests.Logins()
if pr.AssignedActorsUsed {
if issueFeatures.ActorIsAssignable {
editable.Assignees.ActorAssignees = true
editable.Assignees.Default = pr.AssignedActors.DisplayNames()
editable.Assignees.DefaultLogins = pr.AssignedActors.Logins()

View file

@ -26,11 +26,12 @@ func TestNewCmdEdit(t *testing.T) {
require.NoError(t, err)
tests := []struct {
name string
input string
stdin string
output EditOptions
wantsErr bool
name string
input string
stdin string
output EditOptions
expectedBaseRepo ghrepo.Interface
wantsErr bool
}{
{
name: "no argument",
@ -47,6 +48,16 @@ func TestNewCmdEdit(t *testing.T) {
output: EditOptions{},
wantsErr: true,
},
{
name: "URL argument",
input: "https://example.com/cli/cli/pull/23",
output: EditOptions{
SelectorArg: "https://example.com/cli/cli/pull/23",
Interactive: true,
},
expectedBaseRepo: ghrepo.NewWithHost("cli", "cli", "example.com"),
wantsErr: false,
},
{
name: "pull request number argument",
input: "23",
@ -326,6 +337,15 @@ func TestNewCmdEdit(t *testing.T) {
assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg)
assert.Equal(t, tt.output.Interactive, gotOpts.Interactive)
assert.Equal(t, tt.output.Editable, gotOpts.Editable)
if tt.expectedBaseRepo != nil {
baseRepo, err := gotOpts.BaseRepo()
require.NoError(t, err)
require.True(
t,
ghrepo.IsSame(tt.expectedBaseRepo, baseRepo),
"expected base repo %+v, got %+v", tt.expectedBaseRepo, baseRepo,
)
}
})
}
}
@ -344,8 +364,7 @@ func Test_editRun(t *testing.T) {
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "123",
Finder: shared.NewMockFinder("123", &api.PullRequest{
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActorsUsed: true,
URL: "https://github.com/OWNER/REPO/pull/123",
}, ghrepo.New("OWNER", "REPO")),
Interactive: false,
Editable: shared.Editable{
@ -408,8 +427,7 @@ func Test_editRun(t *testing.T) {
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "123",
Finder: shared.NewMockFinder("123", &api.PullRequest{
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActorsUsed: true,
URL: "https://github.com/OWNER/REPO/pull/123",
}, ghrepo.New("OWNER", "REPO")),
Interactive: false,
Editable: shared.Editable{
@ -466,8 +484,7 @@ func Test_editRun(t *testing.T) {
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "123",
Finder: shared.NewMockFinder("123", &api.PullRequest{
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActorsUsed: true,
URL: "https://github.com/OWNER/REPO/pull/123",
}, ghrepo.New("OWNER", "REPO")),
Interactive: false,
Editable: shared.Editable{
@ -529,8 +546,7 @@ func Test_editRun(t *testing.T) {
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "123",
Finder: shared.NewMockFinder("123", &api.PullRequest{
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActorsUsed: true,
URL: "https://github.com/OWNER/REPO/pull/123",
}, ghrepo.New("OWNER", "REPO")),
Interactive: true,
Surveyor: testSurveyor{
@ -576,8 +592,7 @@ func Test_editRun(t *testing.T) {
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "123",
Finder: shared.NewMockFinder("123", &api.PullRequest{
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActorsUsed: true,
URL: "https://github.com/OWNER/REPO/pull/123",
}, ghrepo.New("OWNER", "REPO")),
Interactive: true,
Surveyor: testSurveyor{
@ -620,8 +635,7 @@ func Test_editRun(t *testing.T) {
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "123",
Finder: shared.NewMockFinder("123", &api.PullRequest{
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActorsUsed: true,
URL: "https://github.com/OWNER/REPO/pull/123",
}, ghrepo.New("OWNER", "REPO")),
Interactive: true,
Surveyor: testSurveyor{
@ -667,8 +681,7 @@ func Test_editRun(t *testing.T) {
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "123",
Finder: shared.NewMockFinder("123", &api.PullRequest{
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActorsUsed: true,
URL: "https://github.com/OWNER/REPO/pull/123",
AssignedActors: api.AssignedActors{
Nodes: []api.Actor{
{

View file

@ -112,7 +112,7 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
return nil, nil, errors.New("Find error: no fields specified")
}
if repo, prNumber, err := f.parseURL(opts.Selector); err == nil {
if repo, prNumber, err := ParseURL(opts.Selector); err == nil {
f.prNumber = prNumber
f.baseRefRepo = repo
}
@ -242,33 +242,6 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
}
}
// Ok this is super, super horrible so bear with me.
// The `assignees` field on a Pull Request exposes users that are assigned. It is also possible for bots to be
// assigned, but they only appear under the `assignedActors` field. Ideally, the caller of `Find` would determine
// the correct field to use based on the `fd.Detector` that is passed in, but they can't construct a detector
// because the BaseRepo is only determined within this function. The more correct solution is to do what I did with
// the issue commands and decouple argument parsing from API lookup. See PR #10811 for example.
var actorAssigneesUsed bool
if fields.Contains("assignees") {
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost())
}
issueFeatures, err := opts.Detector.IssueFeatures()
if err != nil {
return nil, nil, fmt.Errorf("error detecting issue features: %v", err)
}
// If actors are assignable on this host then we additionally request the `assignedActors` field.
// Note that we don't remove the `assignees` field because some commands (`pr view`) do not display actor
// assignees yet, so we have to have both sets of data.
if issueFeatures.ActorIsAssignable {
fields.Add("assignedActors")
actorAssigneesUsed = true
}
}
var pr *api.PullRequest
if f.prNumber > 0 {
// If we have a PR number, let's look it up
@ -324,16 +297,14 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
})
}
if actorAssigneesUsed {
pr.AssignedActorsUsed = true
}
return pr, f.baseRefRepo, g.Wait()
}
var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)
func (f *finder) parseURL(prURL string) (ghrepo.Interface, int, error) {
// ParseURL parses a pull request URL and returns the repository and pull
// request number.
func ParseURL(prURL string) (ghrepo.Interface, int, error) {
if prURL == "" {
return nil, 0, fmt.Errorf("invalid URL: %q", prURL)
}

View file

@ -9,12 +9,75 @@ import (
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/require"
)
func TestParseURL(t *testing.T) {
tests := []struct {
name string
arg string
wantRepo ghrepo.Interface
wantNum int
wantErr string
}{
{
name: "valid HTTPS URL",
arg: "https://example.com/owner/repo/pull/123",
wantRepo: ghrepo.NewWithHost("owner", "repo", "example.com"),
wantNum: 123,
},
{
name: "valid HTTP URL",
arg: "http://example.com/owner/repo/pull/123",
wantRepo: ghrepo.NewWithHost("owner", "repo", "example.com"),
wantNum: 123,
},
{
name: "empty URL",
wantErr: "invalid URL: \"\"",
},
{
name: "invalid scheme",
arg: "ftp://github.com/owner/repo/pull/123",
wantErr: "invalid scheme: ftp",
},
{
name: "incorrect path",
arg: "https://github.com/owner/repo/issues/123",
wantErr: "not a pull request URL: https://github.com/owner/repo/issues/123",
},
{
name: "no PR number",
arg: "https://github.com/owner/repo/pull/",
wantErr: "not a pull request URL: https://github.com/owner/repo/pull/",
},
{
name: "invalid PR number",
arg: "https://github.com/owner/repo/pull/foo",
wantErr: "not a pull request URL: https://github.com/owner/repo/pull/foo",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
repo, num, err := ParseURL(tt.arg)
if tt.wantErr != "" {
require.Error(t, err)
require.Equal(t, tt.wantErr, err.Error())
return
}
require.NoError(t, err)
require.Equal(t, tt.wantNum, num)
require.NotNil(t, repo)
require.True(t, ghrepo.IsSame(tt.wantRepo, repo))
})
}
}
type args struct {
baseRepoFn func() (ghrepo.Interface, error)
branchFn func() (string, error)
@ -706,81 +769,6 @@ func TestFind(t *testing.T) {
}
}
func TestFindAssignableActors(t *testing.T) {
t.Run("given actors are not assignable, do nothing special", func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
// Ensure we never request assignedActors
reg.Exclude(t, httpmock.GraphQL(`assignedActors`))
reg.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"pullRequest":{"number":13}
}}}`))
f := finder{
httpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
}
pr, _, err := f.Find(FindOptions{
Detector: &fd.DisabledDetectorMock{},
Fields: []string{"assignees"},
Selector: "https://github.com/cli/cli/pull/13",
})
require.NoError(t, err)
require.False(t, pr.AssignedActorsUsed, "expected PR not to have assigned actors used")
})
t.Run("given actors are assignable, request assignedActors and indicate that on the returned PR", func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
// Ensure that we only respond if assignedActors is requested
reg.Register(
httpmock.GraphQL(`assignedActors`),
httpmock.StringResponse(`{"data":{"repository":{
"pullRequest":{
"number":13,
"assignedActors": {
"nodes": [
{
"id": "HUBOTID",
"login": "hubot",
"__typename": "Bot"
},
{
"id": "MONAID",
"login": "MonaLisa",
"name": "Mona Display Name",
"__typename": "User"
}
],
"totalCount": 2
}}
}}}`))
f := finder{
httpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
}
pr, _, err := f.Find(FindOptions{
Detector: &fd.EnabledDetectorMock{},
Fields: []string{"assignees"},
Selector: "https://github.com/cli/cli/pull/13",
})
require.NoError(t, err)
require.Equal(t, []string{"hubot", "MonaLisa"}, pr.AssignedActors.Logins())
require.True(t, pr.AssignedActorsUsed, "expected PR to have assigned actors used")
})
}
func stubBranchConfig(branchConfig git.BranchConfig, err error) func(context.Context, string) (git.BranchConfig, error) {
return func(_ context.Context, branch string) (git.BranchConfig, error) {
return branchConfig, err

View file

@ -1,7 +1,6 @@
package shared
import (
"archive/zip"
"errors"
"fmt"
"net/http"
@ -230,8 +229,6 @@ type Job struct {
CompletedAt time.Time `json:"completed_at"`
URL string `json:"html_url"`
RunID int64 `json:"run_id"`
Log *zip.File
}
type Step struct {
@ -241,8 +238,6 @@ type Step struct {
Number int
StartedAt time.Time `json:"started_at"`
CompletedAt time.Time `json:"completed_at"`
Log *zip.File
}
type Steps []Step

349
pkg/cmd/run/view/logs.go Normal file
View file

@ -0,0 +1,349 @@
package view
import (
"archive/zip"
"errors"
"fmt"
"io"
"net/http"
"regexp"
"slices"
"sort"
"strings"
"unicode/utf16"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/run/shared"
)
type logFetcher interface {
GetLog() (io.ReadCloser, error)
}
type zipLogFetcher struct {
File *zip.File
}
func (f *zipLogFetcher) GetLog() (io.ReadCloser, error) {
return f.File.Open()
}
type apiLogFetcher struct {
httpClient *http.Client
repo ghrepo.Interface
jobID int64
}
func (f *apiLogFetcher) GetLog() (io.ReadCloser, error) {
logURL := fmt.Sprintf("%srepos/%s/actions/jobs/%d/logs",
ghinstance.RESTPrefix(f.repo.RepoHost()), ghrepo.FullName(f.repo), f.jobID)
req, err := http.NewRequest("GET", logURL, nil)
if err != nil {
return nil, err
}
resp, err := f.httpClient.Do(req)
if err != nil {
return nil, err
}
if resp.StatusCode == 404 {
return nil, fmt.Errorf("log not found: %v", f.jobID)
} else if resp.StatusCode != 200 {
return nil, api.HandleHTTPError(resp)
}
return resp.Body, nil
}
// logSegment represents a segment of a log trail, which can be either an entire
// job log or an individual step log.
type logSegment struct {
job *shared.Job
step *shared.Step
fetcher logFetcher
}
// maxAPILogFetchers is the maximum allowed number of API log fetchers that can
// be assigned to log segments. This is a heuristic limit to avoid overwhelming
// the API with too many requests when fetching logs for a run with many jobs or
// steps.
const maxAPILogFetchers = 25
var errTooManyAPILogFetchers = errors.New("too many missing logs")
// populateLogSegments populates log segments from the provided jobs and data
// available in the given ZIP archive map. Any missing logs will be assigned a
// log fetcher that retrieves logs from the API.
//
// For example, if there's no step log available in the ZIP archive, the entire
// job log will be selected as a log segment.
//
// Note that, as heuristic approach, we only allow a limited number of API log
// fetchers to be assigned. This is to avoid overwhelming the API with too many
// requests.
func populateLogSegments(httpClient *http.Client, repo ghrepo.Interface, jobs []shared.Job, zlm *zipLogMap, onlyFailed bool) ([]logSegment, error) {
segments := make([]logSegment, 0, len(jobs))
apiLogFetcherCount := 0
for _, job := range jobs {
if onlyFailed && !shared.IsFailureState(job.Conclusion) {
continue
}
stepLogAvailable := slices.ContainsFunc(job.Steps, func(step shared.Step) bool {
_, ok := zlm.forStep(job.ID, step.Number)
return ok
})
// If at least one step log is available, we populate the segments with
// them and don't use the entire job log.
if stepLogAvailable {
steps := slices.Clone(job.Steps)
sort.Sort(steps)
for _, step := range steps {
if onlyFailed && !shared.IsFailureState(step.Conclusion) {
continue
}
zf, ok := zlm.forStep(job.ID, step.Number)
if !ok {
// We have no step log in the zip archive, but there's nothing we can do
// about that because there is no API endpoint to fetch step logs.
continue
}
segments = append(segments, logSegment{
job: &job,
step: &step,
fetcher: &zipLogFetcher{File: zf},
})
}
continue
}
segment := logSegment{job: &job}
if zf, ok := zlm.forJob(job.ID); ok {
segment.fetcher = &zipLogFetcher{File: zf}
} else {
segment.fetcher = &apiLogFetcher{
httpClient: httpClient,
repo: repo,
jobID: job.ID,
}
apiLogFetcherCount++
}
segments = append(segments, segment)
if apiLogFetcherCount > maxAPILogFetchers {
return nil, errTooManyAPILogFetchers
}
}
return segments, nil
}
// zipLogMap is a map of job and step logs available in a ZIP archive.
type zipLogMap struct {
jobs map[int64]*zip.File
steps map[string]*zip.File
}
func newZipLogMap() *zipLogMap {
return &zipLogMap{
jobs: make(map[int64]*zip.File),
steps: make(map[string]*zip.File),
}
}
func (l *zipLogMap) forJob(jobID int64) (*zip.File, bool) {
f, ok := l.jobs[jobID]
return f, ok
}
func (l *zipLogMap) forStep(jobID int64, stepNumber int) (*zip.File, bool) {
logFetcherKey := fmt.Sprintf("%d/%d", jobID, stepNumber)
f, ok := l.steps[logFetcherKey]
return f, ok
}
func (l *zipLogMap) addStep(jobID int64, stepNumber int, zf *zip.File) {
logFetcherKey := fmt.Sprintf("%d/%d", jobID, stepNumber)
l.steps[logFetcherKey] = zf
}
func (l *zipLogMap) addJob(jobID int64, zf *zip.File) {
l.jobs[jobID] = zf
}
// getZipLogMap populates a logs struct with appropriate log fetchers based on
// the provided zip file and list of jobs.
//
// The structure of zip file is expected to be as:
//
// zip/
// ├── jobname1/
// │ ├── 1_stepname.txt
// │ ├── 2_anotherstepname.txt
// │ ├── 3_stepstepname.txt
// │ └── 4_laststepname.txt
// ├── jobname2/
// | ├── 1_stepname.txt
// | └── 2_somestepname.txt
// ├── 0_jobname1.txt
// ├── 1_jobname2.txt
// └── -9999999999_jobname3.txt
//
// The function iterates through the list of jobs and tries to find the matching
// log file in the ZIP archive.
//
// The top-level .txt files include the logs for an entire job run. Note that
// the prefixed number is either:
// - An ordinal and cannot be mapped to the corresponding job's ID.
// - A negative integer which is the ID of the job in the old Actions service.
// The service right now tries to get logs and use an ordinal in a loop.
// However, if it doesn't get the logs, it falls back to an old service
// where the ID can apparently be negative.
func getZipLogMap(rlz *zip.Reader, jobs []shared.Job) *zipLogMap {
zlm := newZipLogMap()
for _, job := range jobs {
// So far we haven't yet encountered a ZIP containing both top-level job
// logs (i.e. the normal and the legacy .txt files). However, it's still
// possible. Therefore, we prioritise the normal log over the legacy one.
if zf := matchFileInZIPArchive(rlz, jobLogFilenameRegexp(job)); zf != nil {
zlm.addJob(job.ID, zf)
} else if zf := matchFileInZIPArchive(rlz, legacyJobLogFilenameRegexp(job)); zf != nil {
zlm.addJob(job.ID, zf)
}
for _, step := range job.Steps {
if zf := matchFileInZIPArchive(rlz, stepLogFilenameRegexp(job, step)); zf != nil {
zlm.addStep(job.ID, step.Number, zf)
}
}
}
return zlm
}
const JOB_NAME_MAX_LENGTH = 90
func getJobNameForLogFilename(name string) string {
// As described in https://github.com/cli/cli/issues/5011#issuecomment-1570713070, there are a number of steps
// the server can take when producing the downloaded zip file that can result in a mismatch between the job name
// and the filename in the zip including:
// * Removing characters in the job name that aren't allowed in file paths
// * Truncating names that are too long for zip files
// * Adding collision deduplicating numbers for jobs with the same name
//
// We are hesitant to duplicate all the server logic due to the fragility but it may be unavoidable. Currently, we:
// * Strip `/` which occur when composite action job names are constructed of the form `<JOB_NAME`> / <ACTION_NAME>`
// * Truncate long job names
//
sanitizedJobName := strings.ReplaceAll(name, "/", "")
sanitizedJobName = strings.ReplaceAll(sanitizedJobName, ":", "")
sanitizedJobName = truncateAsUTF16(sanitizedJobName, JOB_NAME_MAX_LENGTH)
return sanitizedJobName
}
// A job run log file is a top-level .txt file whose name starts with an ordinal
// number; e.g., "0_jobname.txt".
func jobLogFilenameRegexp(job shared.Job) *regexp.Regexp {
sanitizedJobName := getJobNameForLogFilename(job.Name)
re := fmt.Sprintf(`^\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName))
return regexp.MustCompile(re)
}
// A legacy job run log file is a top-level .txt file whose name starts with a
// negative number which is the ID of the run; e.g., "-2147483648_jobname.txt".
func legacyJobLogFilenameRegexp(job shared.Job) *regexp.Regexp {
sanitizedJobName := getJobNameForLogFilename(job.Name)
re := fmt.Sprintf(`^-\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName))
return regexp.MustCompile(re)
}
func stepLogFilenameRegexp(job shared.Job, step shared.Step) *regexp.Regexp {
sanitizedJobName := getJobNameForLogFilename(job.Name)
re := fmt.Sprintf(`^%s\/%d_.*\.txt$`, regexp.QuoteMeta(sanitizedJobName), step.Number)
return regexp.MustCompile(re)
}
/*
If you're reading this comment by necessity, I'm sorry and if you're reading it for fun, you're welcome, you weirdo.
What is the length of this string "a😅😅"? If you said 9 you'd be right. If you said 3 or 5 you might also be right!
Here's a summary:
"a" takes 1 byte (`\x61`)
"😅" takes 4 `bytes` (`\xF0\x9F\x98\x85`)
"a😅😅" therefore takes 9 `bytes`
In Go `len("a😅😅")` is 9 because the `len` builtin counts `bytes`
In Go `len([]rune("a😅😅"))` is 3 because each `rune` is 4 `bytes` so each character fits within a `rune`
In C# `"a😅😅".Length` is 5 because `.Length` counts `Char` objects, `Chars` hold 2 bytes, and "😅" takes 2 Chars.
But wait, what does C# have to do with anything? Well the server is running C#. Which server? The one that serves log
files to us in `.zip` format of course! When the server is constructing the zip file to avoid running afoul of a 260
byte zip file path length limitation, it applies transformations to various strings in order to limit their length.
In C#, the server truncates strings with this function:
public static string TruncateAfter(string str, int max)
{
string result = str.Length > max ? str.Substring(0, max) : str;
result = result.Trim();
return result;
}
This seems like it would be easy enough to replicate in Go but as we already discovered, the length of a string isn't
as obvious as it might seem. Since C# uses UTF-16 encoding for strings, and Go uses UTF-8 encoding and represents
characters by runes (which are an alias of int32) we cannot simply slice the string without any further consideration.
Instead, we need to encode the string as UTF-16 bytes, slice it and then decode it back to UTF-8.
Interestingly, in C# length and substring both act on the Char type so it's possible to slice into the middle of
a visual, "representable" character. For example we know `"a😅😅".Length` = 5 (1+2+2) and therefore Substring(0,4)
results in the final character being cleaved in two, resulting in "a😅<61>". Since our int32 runes are being encoded as
2 uint16 elements, we also mimic this behaviour by slicing into the UTF-16 encoded string.
Here's a program you can put into a dotnet playground to see how C# works:
using System;
public class Program {
public static void Main() {
string s = "a😅😅";
Console.WriteLine("{0} {1}", s.Length, s);
string t = TruncateAfter(s, 4);
Console.WriteLine("{0} {1}", t.Length, t);
}
public static string TruncateAfter(string str, int max) {
string result = str.Length > max ? str.Substring(0, max) : str;
return result.Trim();
}
}
This will output:
5 a😅😅
4 a😅<EFBFBD>
*/
func truncateAsUTF16(str string, max int) string {
// Encode the string to UTF-16 to count code units
utf16Encoded := utf16.Encode([]rune(str))
if len(utf16Encoded) > max {
// Decode back to UTF-8 up to the max length
str = string(utf16.Decode(utf16Encoded[:max]))
}
return strings.TrimSpace(str)
}
func matchFileInZIPArchive(zr *zip.Reader, re *regexp.Regexp) *zip.File {
for _, file := range zr.File {
if re.MatchString(file.Name) {
return file
}
}
return nil
}

View file

@ -0,0 +1,542 @@
package view
import (
"archive/zip"
"bytes"
"io"
"net/http"
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/run/shared"
"github.com/cli/cli/v2/pkg/httpmock"
ghAPI "github.com/cli/go-gh/v2/pkg/api"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestZipLogFetcher(t *testing.T) {
zr := createZipReader(t, map[string]string{
"foo.txt": "blah blah",
})
fetcher := &zipLogFetcher{
File: zr.File[0],
}
rc, err := fetcher.GetLog()
assert.NoError(t, err)
defer rc.Close()
content, err := io.ReadAll(rc)
assert.NoError(t, err)
assert.Equal(t, "blah blah", string(content))
}
func TestApiLogFetcher(t *testing.T) {
tests := []struct {
name string
httpStubs func(reg *httpmock.Registry)
wantErr string
wantContent string
}{
{
// This is the real flow as of now. When we call the `/logs`
// endpoint, the server will respond with a 302 redirect, pointing
// to the actual log file URL.
name: "successful with redirect (HTTP 302, then HTTP 200)",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"),
httpmock.WithHeader(
httpmock.StatusStringResponse(http.StatusFound, ""),
"Location",
"https://some.domain/the-actual-log",
),
)
reg.Register(
httpmock.REST("GET", "the-actual-log"),
httpmock.StringResponse("blah blah"),
)
},
wantContent: "blah blah",
},
{
name: "successful without redirect (HTTP 200)",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"),
httpmock.StatusStringResponse(http.StatusOK, "blah blah"),
)
},
wantContent: "blah blah",
},
{
name: "failed with not found error (HTTP 404)",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"),
httpmock.StatusStringResponse(http.StatusNotFound, ""),
)
},
wantErr: "log not found: 123",
},
{
name: "failed with server error (HTTP 500)",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"),
httpmock.JSONErrorResponse(http.StatusInternalServerError, ghAPI.HTTPError{
Message: "blah blah",
StatusCode: http.StatusInternalServerError,
}),
)
},
wantErr: "HTTP 500: blah blah (https://api.github.com/repos/OWNER/REPO/actions/jobs/123/logs)",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.httpStubs(reg)
httpClient := &http.Client{Transport: reg}
fetcher := &apiLogFetcher{
httpClient: httpClient,
repo: ghrepo.New("OWNER", "REPO"),
jobID: 123,
}
rc, err := fetcher.GetLog()
if tt.wantErr != "" {
assert.EqualError(t, err, tt.wantErr)
assert.Nil(t, rc)
return
}
assert.NoError(t, err)
assert.NotNil(t, rc)
content, err := io.ReadAll(rc)
assert.NoError(t, err)
assert.NoError(t, rc.Close())
assert.Equal(t, tt.wantContent, string(content))
})
}
}
func TestGetZipLogMap(t *testing.T) {
tests := []struct {
name string
job shared.Job
zipReader *zip.Reader
// wantJobLog can be nil (i.e. not found) or string
wantJobLog any
// wantStepLogs elements can be nil (i.e. not found) or string
wantStepLogs []any
}{
{
name: "job log missing from zip, but step log present",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"job foo/1_step one.txt": "step one log",
}),
wantJobLog: nil,
wantStepLogs: []any{
"step one log",
},
},
{
name: "matching job name and step number 1",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_job foo.txt": "job log",
"job foo/1_step one.txt": "step one log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
"step one log",
},
},
{
name: "matching job name and step number 2",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step two",
Number: 2,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_job foo.txt": "job log",
"job foo/2_step two.txt": "step two log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
"step two log",
},
},
{
// We should just look for the step number and not the step name.
name: "matching job name and step number and mismatch step name",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "mismatch",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_job foo.txt": "job log",
"job foo/1_step one.txt": "step one log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
"step one log",
},
},
{
name: "matching job name and mismatch step number",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step two",
Number: 2,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_job foo.txt": "job log",
"job foo/1_step one.txt": "step one log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
nil, // no log for step 2
},
},
{
name: "matching job name with no step logs in zip",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_job foo.txt": "job log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "matching job name with no step data",
job: shared.Job{
ID: 123,
Name: "job foo",
},
zipReader: createZipReader(t, map[string]string{
"0_job foo.txt": "job log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "matching job name with random prefix and no step logs in zip",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"999999999_job foo.txt": "job log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "matching job name with legacy filename and no step logs in zip",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"-9999999999_job foo.txt": "job log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "matching job name with legacy filename and no step data",
job: shared.Job{
ID: 123,
Name: "job foo",
},
zipReader: createZipReader(t, map[string]string{
"-9999999999_job foo.txt": "job log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "matching job name with both normal and legacy filename",
job: shared.Job{
ID: 123,
Name: "job foo",
},
zipReader: createZipReader(t, map[string]string{
"0_job foo.txt": "job log",
"-9999999999_job foo.txt": "legacy job log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "one job name is a suffix of another",
job: shared.Job{
ID: 123,
Name: "job foo",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_jjob foo.txt": "the other job log",
"jjob foo/1_step one.txt": "the other step one log",
"1_job foo.txt": "job log",
"job foo/1_step one.txt": "step one log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
"step one log",
},
},
{
name: "escape metacharacters in job name",
job: shared.Job{
ID: 123,
Name: "metacharacters .+*?()|[]{}^$ job",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, nil),
wantJobLog: nil,
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "mismatching job name",
job: shared.Job{
ID: 123,
Name: "mismatch",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, nil),
wantJobLog: nil,
wantStepLogs: []any{
nil, // no log for step 1
},
},
{
name: "job name with forward slash matches dir with slash removed",
job: shared.Job{
ID: 123,
Name: "job foo / with slash",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_job foo with slash.txt": "job log",
"job foo with slash/1_step one.txt": "step one log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
"step one log",
},
},
{
name: "job name with colon matches dir with colon removed",
job: shared.Job{
ID: 123,
Name: "job foo : with colon",
Steps: []shared.Step{{
Name: "step one",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"0_job foo with colon.txt": "job log",
"job foo with colon/1_step one.txt": "step one log",
}),
wantJobLog: "job log",
wantStepLogs: []any{
"step one log",
},
},
{
name: "job name with really long name (over the ZIP limit)",
job: shared.Job{
ID: 123,
Name: "thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_",
Steps: []shared.Step{{
Name: "long name job",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnine/1_long name job.txt": "step one log",
}),
wantJobLog: nil,
wantStepLogs: []any{
"step one log",
},
},
{
name: "job name that would be truncated by the C# server to split a grapheme",
job: shared.Job{
ID: 123,
Name: "emoji test 😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅",
Steps: []shared.Step{{
Name: "emoji job",
Number: 1,
}},
},
zipReader: createZipReader(t, map[string]string{
"emoji test 😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅<F09F9885>/1_emoji job.txt": "step one log",
}),
wantJobLog: nil,
wantStepLogs: []any{
"step one log",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
logMap := getZipLogMap(tt.zipReader, []shared.Job{tt.job})
jobLogFile, ok := logMap.forJob(tt.job.ID)
switch want := tt.wantJobLog.(type) {
case nil:
require.False(t, ok)
require.Nil(t, jobLogFile)
case string:
require.True(t, ok)
require.NotNil(t, jobLogFile)
require.Equal(t, want, string(readZipFile(t, jobLogFile)))
default:
t.Fatal("wantJobLog must be nil or string")
}
for i, wantStepLog := range tt.wantStepLogs {
stepLogFile, ok := logMap.forStep(tt.job.ID, 1+i) // Step numbers start from 1
switch want := wantStepLog.(type) {
case nil:
require.False(t, ok)
require.Nil(t, stepLogFile)
case string:
require.True(t, ok)
require.NotNil(t, stepLogFile)
gotStepLog := readZipFile(t, stepLogFile)
require.Equal(t, want, string(gotStepLog))
default:
t.Fatal("wantStepLog must be nil or string")
}
}
})
}
}
func readZipFile(t *testing.T, zf *zip.File) []byte {
rc, err := zf.Open()
assert.NoError(t, err)
defer rc.Close()
content, err := io.ReadAll(rc)
assert.NoError(t, err)
return content
}
func createZipReader(t *testing.T, files map[string]string) *zip.Reader {
raw := createZipArchive(t, files)
zr, err := zip.NewReader(bytes.NewReader(raw), int64(len(raw)))
assert.NoError(t, err)
return zr
}
func createZipArchive(t *testing.T, files map[string]string) []byte {
buf := bytes.NewBuffer(nil)
zw := zip.NewWriter(buf)
for name, content := range files {
fileWriter, err := zw.Create(name)
assert.NoError(t, err)
_, err = fileWriter.Write([]byte(content))
assert.NoError(t, err)
}
err := zw.Close()
assert.NoError(t, err)
return buf.Bytes()
}

View file

@ -10,12 +10,8 @@ import (
"net/http"
"os"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"time"
"unicode/utf16"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
@ -116,13 +112,17 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
Long: heredoc.Docf(`
View a summary of a workflow run.
This command does not support authenticating via fine grained PATs
as it is not currently possible to create a PAT with the %[1]schecks:read%[1]s permission.
Due to platform limitations, %[1]sgh%[1]s may not always be able to associate jobs with their
corresponding logs when using the primary method of fetching logs in zip format.
Due to platform limitations, %[1]sgh%[1]s may not always be able to associate log lines with a
particular step in a job. In this case, the step name in the log output will be replaced with
%[1]sUNKNOWN STEP%[1]s.
`, "`"),
In such cases, %[1]sgh%[1]s will attempt to fetch logs for each job individually via the API.
This fallback is slower and more resource-intensive. If more than 25 job logs are missing,
the operation will fail with an error.
Additionally, due to similar platform constraints, some log lines may not be
associated with a specific step within a job. In these cases, the step name will
appear as %[1]sUNKNOWN STEP%[1]s in the log output.
`, "`", maxAPILogFetchers),
Args: cobra.MaximumNArgs(1),
Example: heredoc.Doc(`
# Interactively select a run to view, optionally selecting a single job
@ -322,9 +322,16 @@ func runView(opts *ViewOptions) error {
}
defer runLogZip.Close()
attachRunLog(&runLogZip.Reader, jobs)
zlm := getZipLogMap(&runLogZip.Reader, jobs)
segments, err := populateLogSegments(httpClient, repo, jobs, zlm, opts.LogFailed)
if err != nil {
if errors.Is(err, errTooManyAPILogFetchers) {
return fmt.Errorf("too many API requests needed to fetch logs; try narrowing down to a specific job with the `--job` option")
}
return err
}
return displayRunLog(opts.IO.Out, jobs, opts.LogFailed)
return displayLogSegments(opts.IO.Out, segments)
}
prNumber := ""
@ -535,212 +542,28 @@ func promptForJob(prompter shared.Prompter, cs *iostreams.ColorScheme, jobs []sh
return nil, nil
}
const JOB_NAME_MAX_LENGTH = 90
func getJobNameForLogFilename(name string) string {
// As described in https://github.com/cli/cli/issues/5011#issuecomment-1570713070, there are a number of steps
// the server can take when producing the downloaded zip file that can result in a mismatch between the job name
// and the filename in the zip including:
// * Removing characters in the job name that aren't allowed in file paths
// * Truncating names that are too long for zip files
// * Adding collision deduplicating numbers for jobs with the same name
//
// We are hesitant to duplicate all the server logic due to the fragility but it may be unavoidable. Currently, we:
// * Strip `/` which occur when composite action job names are constructed of the form `<JOB_NAME`> / <ACTION_NAME>`
// * Truncate long job names
//
sanitizedJobName := strings.ReplaceAll(name, "/", "")
sanitizedJobName = strings.ReplaceAll(sanitizedJobName, ":", "")
sanitizedJobName = truncateAsUTF16(sanitizedJobName, JOB_NAME_MAX_LENGTH)
return sanitizedJobName
}
// A job run log file is a top-level .txt file whose name starts with an ordinal
// number; e.g., "0_jobname.txt".
func jobLogFilenameRegexp(job shared.Job) *regexp.Regexp {
sanitizedJobName := getJobNameForLogFilename(job.Name)
re := fmt.Sprintf(`^\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName))
return regexp.MustCompile(re)
}
// A legacy job run log file is a top-level .txt file whose name starts with a
// negative number which is the ID of the run; e.g., "-2147483648_jobname.txt".
func legacyJobLogFilenameRegexp(job shared.Job) *regexp.Regexp {
sanitizedJobName := getJobNameForLogFilename(job.Name)
re := fmt.Sprintf(`^-\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName))
return regexp.MustCompile(re)
}
func stepLogFilenameRegexp(job shared.Job, step shared.Step) *regexp.Regexp {
sanitizedJobName := getJobNameForLogFilename(job.Name)
re := fmt.Sprintf(`^%s\/%d_.*\.txt$`, regexp.QuoteMeta(sanitizedJobName), step.Number)
return regexp.MustCompile(re)
}
/*
If you're reading this comment by necessity, I'm sorry and if you're reading it for fun, you're welcome, you weirdo.
What is the length of this string "a😅😅"? If you said 9 you'd be right. If you said 3 or 5 you might also be right!
Here's a summary:
"a" takes 1 byte (`\x61`)
"😅" takes 4 `bytes` (`\xF0\x9F\x98\x85`)
"a😅😅" therefore takes 9 `bytes`
In Go `len("a😅😅")` is 9 because the `len` builtin counts `bytes`
In Go `len([]rune("a😅😅"))` is 3 because each `rune` is 4 `bytes` so each character fits within a `rune`
In C# `"a😅😅".Length` is 5 because `.Length` counts `Char` objects, `Chars` hold 2 bytes, and "😅" takes 2 Chars.
But wait, what does C# have to do with anything? Well the server is running C#. Which server? The one that serves log
files to us in `.zip` format of course! When the server is constructing the zip file to avoid running afoul of a 260
byte zip file path length limitation, it applies transformations to various strings in order to limit their length.
In C#, the server truncates strings with this function:
public static string TruncateAfter(string str, int max)
{
string result = str.Length > max ? str.Substring(0, max) : str;
result = result.Trim();
return result;
}
This seems like it would be easy enough to replicate in Go but as we already discovered, the length of a string isn't
as obvious as it might seem. Since C# uses UTF-16 encoding for strings, and Go uses UTF-8 encoding and represents
characters by runes (which are an alias of int32) we cannot simply slice the string without any further consideration.
Instead, we need to encode the string as UTF-16 bytes, slice it and then decode it back to UTF-8.
Interestingly, in C# length and substring both act on the Char type so it's possible to slice into the middle of
a visual, "representable" character. For example we know `"a😅😅".Length` = 5 (1+2+2) and therefore Substring(0,4)
results in the final character being cleaved in two, resulting in "a😅<61>". Since our int32 runes are being encoded as
2 uint16 elements, we also mimic this behaviour by slicing into the UTF-16 encoded string.
Here's a program you can put into a dotnet playground to see how C# works:
using System;
public class Program {
public static void Main() {
string s = "a😅😅";
Console.WriteLine("{0} {1}", s.Length, s);
string t = TruncateAfter(s, 4);
Console.WriteLine("{0} {1}", t.Length, t);
}
public static string TruncateAfter(string str, int max) {
string result = str.Length > max ? str.Substring(0, max) : str;
return result.Trim();
}
}
This will output:
5 a😅😅
4 a😅<EFBFBD>
*/
func truncateAsUTF16(str string, max int) string {
// Encode the string to UTF-16 to count code units
utf16Encoded := utf16.Encode([]rune(str))
if len(utf16Encoded) > max {
// Decode back to UTF-8 up to the max length
str = string(utf16.Decode(utf16Encoded[:max]))
}
return strings.TrimSpace(str)
}
// This function takes a zip file of logs and a list of jobs.
// Structure of zip file
//
// zip/
// ├── jobname1/
// │ ├── 1_stepname.txt
// │ ├── 2_anotherstepname.txt
// │ ├── 3_stepstepname.txt
// │ └── 4_laststepname.txt
// ├── jobname2/
// | ├── 1_stepname.txt
// | └── 2_somestepname.txt
// ├── 0_jobname1.txt
// ├── 1_jobname2.txt
// └── -9999999999_jobname3.txt
//
// It iterates through the list of jobs and tries to find the matching
// log in the zip file. If the matching log is found it is attached
// to the job.
//
// The top-level .txt files include the logs for an entire job run. Note that
// the prefixed number is either:
// - An ordinal and cannot be mapped to the corresponding job's ID.
// - A negative integer which is the ID of the job in the old Actions service.
// The service right now tries to get logs and use an ordinal in a loop.
// However, if it doesn't get the logs, it falls back to an old service
// where the ID can apparently be negative.
func attachRunLog(rlz *zip.Reader, jobs []shared.Job) {
for i, job := range jobs {
// As a highest priority, we try to use the step logs first. We have seen zips that surprisingly contain
// step logs, normal job logs and legacy job logs. In this case, both job logs would be ignored. We have
// never seen a zip containing both job logs and no step logs, however, it may be possible. In that case
// let's prioritise the normal log over the legacy one.
jobLog := matchFileInZIPArchive(rlz, jobLogFilenameRegexp(job))
if jobLog == nil {
jobLog = matchFileInZIPArchive(rlz, legacyJobLogFilenameRegexp(job))
func displayLogSegments(w io.Writer, segments []logSegment) error {
for _, segment := range segments {
stepName := "UNKNOWN STEP"
if segment.step != nil {
stepName = segment.step.Name
}
jobs[i].Log = jobLog
for j, step := range job.Steps {
jobs[i].Steps[j].Log = matchFileInZIPArchive(rlz, stepLogFilenameRegexp(job, step))
rc, err := segment.fetcher.GetLog()
if err != nil {
return err
}
}
}
func matchFileInZIPArchive(zr *zip.Reader, re *regexp.Regexp) *zip.File {
for _, file := range zr.File {
if re.MatchString(file.Name) {
return file
}
}
return nil
}
func displayRunLog(w io.Writer, jobs []shared.Job, failed bool) error {
for _, job := range jobs {
// To display a run log, we first try to compile it from individual step
// logs, because this way we can prepend lines with the corresponding
// step name. However, at the time of writing, logs are sometimes being
// served by a service that doesnt include the step logs (none of them),
// in which case we fall back to print the entire job run log.
var hasStepLogs bool
steps := job.Steps
sort.Sort(steps)
for _, step := range steps {
if failed && !shared.IsFailureState(step.Conclusion) {
continue
}
if step.Log == nil {
continue
}
hasStepLogs = true
prefix := fmt.Sprintf("%s\t%s\t", job.Name, step.Name)
if err := printZIPFile(w, step.Log, prefix); err != nil {
err = func() error {
defer rc.Close()
prefix := fmt.Sprintf("%s\t%s\t", segment.job.Name, stepName)
if err := copyLogWithLinePrefix(w, rc, prefix); err != nil {
return err
}
}
return nil
}()
if hasStepLogs {
continue
}
if failed && !shared.IsFailureState(job.Conclusion) {
continue
}
if job.Log == nil {
continue
}
// Here, we fall back to the job run log, which means we do not know
// the step name of lines. However, we want to keep the same line
// formatting to avoid breaking any code or script that rely on the
// tab-delimited formatting. So, an unknown-step placeholder is used
// instead of the actual step name.
prefix := fmt.Sprintf("%s\tUNKNOWN STEP\t", job.Name)
if err := printZIPFile(w, job.Log, prefix); err != nil {
if err != nil {
return err
}
}
@ -748,14 +571,8 @@ func displayRunLog(w io.Writer, jobs []shared.Job, failed bool) error {
return nil
}
func printZIPFile(w io.Writer, file *zip.File, prefix string) error {
f, err := file.Open()
if err != nil {
return err
}
defer f.Close()
scanner := bufio.NewScanner(f)
func copyLogWithLinePrefix(w io.Writer, r io.Reader, prefix string) error {
scanner := bufio.NewScanner(r)
for scanner.Scan() {
fmt.Fprintf(w, "%s%s\n", prefix, scanner.Text())
}

File diff suppressed because it is too large Load diff

View file

@ -53,6 +53,11 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
Prompter: f.Prompter,
}
// It is possible for a user to say `--no-repos-selected=false --repos cli/cli` and that would be equivalent to not
// specifying the flag at all. We could avoid this by checking whether the flag was set at all, but it seems like
// more trouble than it's worth since anyone who does `--no-repos-selected=false` is gonna get what's coming to them.
var noRepositoriesSelected bool
cmd := &cobra.Command{
Use: "set <secret-name>",
Short: "Create or update secrets",
@ -90,6 +95,9 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
# Set organization-level secret visible to specific repositories
$ gh secret set MYSECRET --org myOrg --repos repo1,repo2,repo3
# Set organization-level secret visible to no repositories
$ gh secret set MYSECRET --org myOrg --no-repos-selected
# Set user-level secret for Codespaces
$ gh secret set MYSECRET --user
@ -131,6 +139,14 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
return err
}
if err := cmdutil.MutuallyExclusive("specify only one of `--repos` or `--no-repos-selected`", len(opts.RepositoryNames) > 0, noRepositoriesSelected); err != nil {
return err
}
if err := cmdutil.MutuallyExclusive("`--no-repos-selected` must be omitted when used with `--user`", opts.UserSecrets, noRepositoriesSelected); err != nil {
return err
}
if len(args) == 0 {
if !opts.DoNotStore && opts.EnvFile == "" {
return cmdutil.FlagErrorf("must pass name argument")
@ -148,11 +164,16 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
return cmdutil.FlagErrorf("`--repos` is only supported with `--visibility=selected`")
}
if opts.Visibility == shared.Selected && len(opts.RepositoryNames) == 0 {
return cmdutil.FlagErrorf("`--repos` list required with `--visibility=selected`")
if opts.Visibility != shared.Selected && noRepositoriesSelected {
return cmdutil.FlagErrorf("`--no-repos-selected` is only supported with `--visibility=selected`")
}
if opts.Visibility == shared.Selected && (len(opts.RepositoryNames) == 0 && !noRepositoriesSelected) {
return cmdutil.FlagErrorf("`--repos` or `--no-repos-selected` required with `--visibility=selected`")
}
} else {
if len(opts.RepositoryNames) > 0 {
if len(opts.RepositoryNames) > 0 || noRepositoriesSelected {
opts.Visibility = shared.Selected
}
}
@ -170,6 +191,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
cmd.Flags().BoolVarP(&opts.UserSecrets, "user", "u", false, "Set a secret for your user")
cmdutil.StringEnumFlag(cmd, &opts.Visibility, "visibility", "v", shared.Private, []string{shared.All, shared.Private, shared.Selected}, "Set visibility for an organization secret")
cmd.Flags().StringSliceVarP(&opts.RepositoryNames, "repos", "r", []string{}, "List of `repositories` that can access an organization or user secret")
cmd.Flags().BoolVar(&noRepositoriesSelected, "no-repos-selected", false, "No repositories can access the organization secret")
cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "The value for the secret (reads from standard input if not specified)")
cmd.Flags().BoolVar(&opts.DoNotStore, "no-store", false, "Print the encrypted, base64-encoded value instead of storing it on GitHub")
cmd.Flags().StringVarP(&opts.EnvFile, "env-file", "f", "", "Load secret names and values from a dotenv-formatted `file`")

View file

@ -27,88 +27,120 @@ import (
func TestNewCmdSet(t *testing.T) {
tests := []struct {
name string
cli string
wants SetOptions
stdinTTY bool
wantsErr bool
name string
args string
wants SetOptions
stdinTTY bool
wantsErr bool
wantsErrMessage string
}{
{
name: "invalid visibility",
cli: "cool_secret --org coolOrg -v'mistyVeil'",
args: "cool_secret --org coolOrg -v mistyVeil",
wantsErr: true,
},
{
name: "invalid visibility",
cli: "cool_secret --org coolOrg -v'selected'",
name: "when visibility is selected, requires indication of repos",
args: "cool_secret --org coolOrg -v selected",
wantsErr: true,
wantsErrMessage: "`--repos` or `--no-repos-selected` required with `--visibility=selected`",
},
{
name: "visibilities other than selected do not accept --repos",
args: "cool_secret --org coolOrg -v private -r coolRepo",
wantsErr: true,
wantsErrMessage: "`--repos` is only supported with `--visibility=selected`",
},
{
name: "visibilities other than selected do not accept --no-repos-selected",
args: "cool_secret --org coolOrg -v private --no-repos-selected",
wantsErr: true,
wantsErrMessage: "`--no-repos-selected` is only supported with `--visibility=selected`",
},
{
name: "--repos and --no-repos-selected are mutually exclusive",
args: `--repos coolRepo --no-repos-selected cool_secret`,
wantsErr: true,
wantsErrMessage: "specify only one of `--repos` or `--no-repos-selected`",
},
{
name: "secret name is required",
args: "",
wantsErr: true,
},
{
name: "repos with wrong vis",
cli: "cool_secret --org coolOrg -v'private' -rcoolRepo",
name: "multiple positional arguments are not allowed",
args: "cool_secret good_secret",
wantsErr: true,
},
{
name: "no name",
cli: "",
name: "visibility is only allowed with --org",
args: "cool_secret -v all",
wantsErr: true,
},
{
name: "multiple names",
cli: "cool_secret good_secret",
wantsErr: true,
},
{
name: "visibility without org",
cli: "cool_secret -vall",
wantsErr: true,
},
{
name: "repos without vis",
cli: "cool_secret -bs --org coolOrg -rcoolRepo",
name: "providing --repos without --visibility implies selected visibility",
args: "cool_secret --body secret-body --org coolOrg --repos coolRepo",
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Selected,
RepositoryNames: []string{"coolRepo"},
Body: "s",
Body: "secret-body",
OrgName: "coolOrg",
},
},
{
name: "providing --no-repos-selected without --visibility implies selected visibility",
args: "cool_secret --body secret-body --org coolOrg --no-repos-selected",
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Selected,
RepositoryNames: []string{},
Body: "secret-body",
OrgName: "coolOrg",
},
},
{
name: "org with selected repo",
cli: "-ocoolOrg -bs -vselected -rcoolRepo cool_secret",
args: "-o coolOrg --body secret-body -v selected -r coolRepo cool_secret",
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Selected,
RepositoryNames: []string{"coolRepo"},
Body: "s",
Body: "secret-body",
OrgName: "coolOrg",
},
},
{
name: "org with selected repos",
cli: `--org=coolOrg -bs -vselected -r="coolRepo,radRepo,goodRepo" cool_secret`,
args: `--org coolOrg --body secret-body -v selected --repos "coolRepo,radRepo,goodRepo" cool_secret`,
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Selected,
RepositoryNames: []string{"coolRepo", "goodRepo", "radRepo"},
Body: "s",
Body: "secret-body",
OrgName: "coolOrg",
},
},
{
name: "user with selected repos",
cli: `-u -bs -r"monalisa/coolRepo,cli/cli,github/hub" cool_secret`,
args: `-u --body secret-body -r "monalisa/coolRepo,cli/cli,github/hub" cool_secret`,
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Selected,
RepositoryNames: []string{"monalisa/coolRepo", "cli/cli", "github/hub"},
Body: "s",
Body: "secret-body",
},
},
{
name: "--user is mutually exclusive with --no-repos-selected",
args: `-u --no-repos-selected cool_secret`,
wantsErr: true,
wantsErrMessage: "`--no-repos-selected` must be omitted when used with `--user`",
},
{
name: "repo",
cli: `cool_secret -b"a secret"`,
args: `cool_secret --body "a secret"`,
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Private,
@ -118,7 +150,7 @@ func TestNewCmdSet(t *testing.T) {
},
{
name: "env",
cli: `cool_secret -b"a secret" -eRelease`,
args: `cool_secret --body "a secret" --env Release`,
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Private,
@ -129,7 +161,7 @@ func TestNewCmdSet(t *testing.T) {
},
{
name: "vis all",
cli: `cool_secret --org coolOrg -b"cool" -vall`,
args: `cool_secret --org coolOrg --body "cool" --visibility all`,
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.All,
@ -139,7 +171,7 @@ func TestNewCmdSet(t *testing.T) {
},
{
name: "no store",
cli: `cool_secret --no-store`,
args: `cool_secret --no-store`,
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Private,
@ -148,7 +180,7 @@ func TestNewCmdSet(t *testing.T) {
},
{
name: "Dependabot repo",
cli: `cool_secret -b"a secret" --app Dependabot`,
args: `cool_secret --body "a secret" --app Dependabot`,
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Private,
@ -159,19 +191,19 @@ func TestNewCmdSet(t *testing.T) {
},
{
name: "Dependabot org",
cli: "-ocoolOrg -bs -vselected -rcoolRepo cool_secret -aDependabot",
args: "--org coolOrg --body secret-body --visibility selected --repos coolRepo cool_secret --app Dependabot",
wants: SetOptions{
SecretName: "cool_secret",
Visibility: shared.Selected,
RepositoryNames: []string{"coolRepo"},
Body: "s",
Body: "secret-body",
OrgName: "coolOrg",
Application: "Dependabot",
},
},
{
name: "Codespaces org",
cli: `random_secret -ocoolOrg -b"random value" -vselected -r"coolRepo,cli/cli" -aCodespaces`,
args: `random_secret --org coolOrg --body "random value" --visibility selected --repos "coolRepo,cli/cli" --app Codespaces`,
wants: SetOptions{
SecretName: "random_secret",
Visibility: shared.Selected,
@ -192,7 +224,7 @@ func TestNewCmdSet(t *testing.T) {
ios.SetStdinTTY(tt.stdinTTY)
argv, err := shlex.Split(tt.cli)
argv, err := shlex.Split(tt.args)
assert.NoError(t, err)
var gotOpts *SetOptions
@ -208,6 +240,9 @@ func TestNewCmdSet(t *testing.T) {
_, err = cmd.ExecuteC()
if tt.wantsErr {
assert.Error(t, err)
if tt.wantsErrMessage != "" {
assert.EqualError(t, err, tt.wantsErrMessage)
}
return
}
assert.NoError(t, err)
@ -497,6 +532,16 @@ func Test_setRun_org(t *testing.T) {
wantRepositories: []int64{1, 2},
wantApp: "actions",
},
{
name: "no repos visibility",
opts: &SetOptions{
OrgName: "UmbrellaCorporation",
Visibility: shared.Selected,
RepositoryNames: []string{},
},
wantRepositories: []int64{},
wantApp: "actions",
},
{
name: "Dependabot",
opts: &SetOptions{
@ -517,6 +562,17 @@ func Test_setRun_org(t *testing.T) {
wantDependabotRepositories: []string{"1", "2"},
wantApp: "dependabot",
},
{
name: "Dependabot no repos visibility",
opts: &SetOptions{
OrgName: "UmbrellaCorporation",
Visibility: shared.Selected,
Application: shared.Dependabot,
RepositoryNames: []string{},
},
wantRepositories: []int64{},
wantApp: "dependabot",
},
}
for _, tt := range tests {

View file

@ -127,6 +127,12 @@ func StringResponse(body string) Responder {
}
}
func BinaryResponse(body []byte) Responder {
return func(req *http.Request) (*http.Response, error) {
return httpResponse(200, req, bytes.NewBuffer(body)), nil
}
}
func WithHost(matcher Matcher, host string) Matcher {
return func(req *http.Request) bool {
if !strings.EqualFold(req.Host, host) {

View file

@ -106,7 +106,7 @@ Some packages may only be included on certain architectures or operating systems
- [github.com/mattn/go-runewidth](https://pkg.go.dev/github.com/mattn/go-runewidth) ([MIT](https://github.com/mattn/go-runewidth/blob/v0.0.16/LICENSE))
- [github.com/mgutz/ansi](https://pkg.go.dev/github.com/mgutz/ansi) ([MIT](https://github.com/mgutz/ansi/blob/d51e80ef957d/LICENSE))
- [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md))
- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.0.25/LICENSE))
- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.1.13/LICENSE))
- [github.com/mitchellh/copystructure](https://pkg.go.dev/github.com/mitchellh/copystructure) ([MIT](https://github.com/mitchellh/copystructure/blob/v1.2.0/LICENSE))
- [github.com/mitchellh/go-homedir](https://pkg.go.dev/github.com/mitchellh/go-homedir) ([MIT](https://github.com/mitchellh/go-homedir/blob/v1.1.0/LICENSE))
- [github.com/mitchellh/hashstructure/v2](https://pkg.go.dev/github.com/mitchellh/hashstructure/v2) ([MIT](https://github.com/mitchellh/hashstructure/blob/v2.0.2/LICENSE))

View file

@ -106,7 +106,7 @@ Some packages may only be included on certain architectures or operating systems
- [github.com/mattn/go-runewidth](https://pkg.go.dev/github.com/mattn/go-runewidth) ([MIT](https://github.com/mattn/go-runewidth/blob/v0.0.16/LICENSE))
- [github.com/mgutz/ansi](https://pkg.go.dev/github.com/mgutz/ansi) ([MIT](https://github.com/mgutz/ansi/blob/d51e80ef957d/LICENSE))
- [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md))
- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.0.25/LICENSE))
- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.1.13/LICENSE))
- [github.com/mitchellh/copystructure](https://pkg.go.dev/github.com/mitchellh/copystructure) ([MIT](https://github.com/mitchellh/copystructure/blob/v1.2.0/LICENSE))
- [github.com/mitchellh/go-homedir](https://pkg.go.dev/github.com/mitchellh/go-homedir) ([MIT](https://github.com/mitchellh/go-homedir/blob/v1.1.0/LICENSE))
- [github.com/mitchellh/hashstructure/v2](https://pkg.go.dev/github.com/mitchellh/hashstructure/v2) ([MIT](https://github.com/mitchellh/hashstructure/blob/v2.0.2/LICENSE))

View file

@ -109,7 +109,7 @@ Some packages may only be included on certain architectures or operating systems
- [github.com/mattn/go-runewidth](https://pkg.go.dev/github.com/mattn/go-runewidth) ([MIT](https://github.com/mattn/go-runewidth/blob/v0.0.16/LICENSE))
- [github.com/mgutz/ansi](https://pkg.go.dev/github.com/mgutz/ansi) ([MIT](https://github.com/mgutz/ansi/blob/d51e80ef957d/LICENSE))
- [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md))
- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.0.25/LICENSE))
- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.1.13/LICENSE))
- [github.com/mitchellh/copystructure](https://pkg.go.dev/github.com/mitchellh/copystructure) ([MIT](https://github.com/mitchellh/copystructure/blob/v1.2.0/LICENSE))
- [github.com/mitchellh/go-homedir](https://pkg.go.dev/github.com/mitchellh/go-homedir) ([MIT](https://github.com/mitchellh/go-homedir/blob/v1.1.0/LICENSE))
- [github.com/mitchellh/hashstructure/v2](https://pkg.go.dev/github.com/mitchellh/hashstructure/v2) ([MIT](https://github.com/mitchellh/hashstructure/blob/v2.0.2/LICENSE))