Skip to content

Commit 50584dd

Browse files
authored
Merge branch 'main' into peterguy/support-http_proxy
2 parents 8ae0bdc + 284f08b commit 50584dd

48 files changed

Lines changed: 1899 additions & 844 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
name: writing-prs-and-commits
3+
description: Writes and revises Sourcegraph pull request titles, descriptions, and single-PR commit messages. Use when creating or updating a pull request, or when preparing a commit that should stand on its own as the eventual PR.
4+
---
5+
6+
# Writing PRs and commits
7+
8+
Use this skill when the user asks to create a PR, update an existing PR title or body, or write a commit message for a change that maps cleanly to one PR.
9+
10+
## Resolve the target
11+
12+
- If the user names a PR number or URL, use that.
13+
- Otherwise, inspect the current branch and use `gh pr view --json number,title,body,baseRefName,headRefName,url` to find the associated PR.
14+
- If no PR exists and the user wants one created, use `gh pr create`, then immediately verify or adjust the title and body.
15+
- If the user wants text only, provide the draft without mutating GitHub.
16+
- When a PR is part of a stack, describe only the net change between the PR head and its base branch. Do not describe the full stack relative to `main`.
17+
18+
## Core writing rules
19+
20+
- Lead with _why_. The first paragraph or sentence should explain the problem, motivation, or user impact.
21+
- Follow with _what_ changed. Once the reader understands why the change exists, describe the solution and any important constraints or tradeoffs.
22+
- Write the net change only. Do not document abandoned experiments, reverted approaches, or temporary dead ends.
23+
- Prefer natural prose over rigid `Summary` / `Problem` / `Solution` headings unless the user explicitly asks for that format.
24+
- Keep references repo-relative. Do not mention absolute local filesystem paths.
25+
- Preserve valuable existing PR content such as screenshots, images, rollout notes, and manually written context unless the user asks to remove it.
26+
- Use Markdown cleanly: inline code in backticks, fenced blocks when needed, and GitHub permalinks when citing specific code.
27+
28+
## PR titles
29+
30+
- Keep titles short, specific, and verb-first.
31+
- Prefer Sourcegraph's changelog-friendly shape when it fits the change: `type/domain: title`.
32+
- Use conventional prefixes like `feat`, `fix`, `remove`, and `chore` when they match the change.
33+
- Make the title describe the improvement or behavior change, not just the bug report.
34+
- If the PR is user-visible and likely to feed the changelog, choose a domain that reflects the product or feature area rather than an implementation detail.
35+
36+
## PR descriptions
37+
38+
- Write the opening prose in this order:
39+
1. Why this change is needed.
40+
2. What changed.
41+
3. Any important tradeoffs, rollout notes, or follow-up context.
42+
- When relevant, reference linked Linear issues or related Slack threads.
43+
- If docs or handbook follow-up is needed, mention it in the prose before the verification section.
44+
- Include `## Test Plan` near the end when there is meaningful validation to report.
45+
- The test plan should mention intentional verification such as focused tests, manual flows, or reproduction steps. Do not pad it with routine formatting or CI work the repository already expects.
46+
- Include `## Changelog` only for end-user-visible changes worth communicating outside the PR. Keep it as the last section. Omit it for refactors, tooling, internal cleanup, or other changes with no user-facing impact.
47+
48+
## Commit messages for single-PR changes
49+
50+
Treat a single commit that is intended to become one PR as a PR. GitHub by default will use the title and body from the commit.
51+
52+
## Workflow
53+
54+
1. Inspect the diff, linked context, and any existing PR title, body, or commit message.
55+
2. Decide whether you are creating a new PR, editing an existing PR, or writing a stand-alone commit message.
56+
3. Draft the title and body so the motivation is clear before the implementation details.
57+
4. Check that the text matches the net change, preserves important existing content, and follows Sourcegraph conventions for `Test Plan` and `Changelog`.
58+
5. Apply the final text with `gh pr create`, `gh pr edit`, or `git commit` as requested.

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@ bazel-zoekt
1414
bazel-src-cli
1515
.DS_Store
1616
samples
17+
.amp

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Command Guidelines
2+
3+
- When adding new commands, use `urfave/cli` instead of the legacy `commander` pattern.
4+
- Register new `urfave/cli` commands in the `migratedCommands` map in `cmd/src/run_migration_compat.go`.

cmd/src/abc.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package main
2+
3+
import (
4+
"github.com/sourcegraph/src-cli/internal/clicompat"
5+
"github.com/urfave/cli/v3"
6+
)
7+
8+
var abcCommand = clicompat.Wrap(&cli.Command{
9+
Name: "abc",
10+
Usage: "manages agentic batch changes",
11+
Commands: []*cli.Command{
12+
clicompat.Wrap(&cli.Command{
13+
Name: "variables",
14+
Usage: "manage workflow instance variables",
15+
Commands: []*cli.Command{
16+
abcVariablesSetCommand,
17+
abcVariablesDeleteCommand,
18+
},
19+
}),
20+
},
21+
})

cmd/src/abc_variables_delete.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"io"
7+
"slices"
8+
9+
"github.com/sourcegraph/src-cli/internal/api"
10+
"github.com/sourcegraph/src-cli/internal/clicompat"
11+
"github.com/sourcegraph/src-cli/internal/cmderrors"
12+
"github.com/urfave/cli/v3"
13+
)
14+
15+
var abcVariablesDeleteCommand = clicompat.Wrap(&cli.Command{
16+
Name: "delete",
17+
Usage: "Delete variables on a workflow instance",
18+
UsageText: "src abc variables delete [options] <workflow-instance-id> [<name> ...]",
19+
DisableSliceFlagSeparator: true,
20+
Description: `
21+
Delete workflow instance variables
22+
23+
Examples:
24+
25+
Delete a variable from a workflow instance:
26+
27+
$ src abc variables delete QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ== approval
28+
29+
Delete multiple variables in one request:
30+
31+
$ src abc variables delete QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ== --var approval --var checkpoints
32+
`,
33+
Flags: clicompat.WithAPIFlags(
34+
&cli.StringSliceFlag{
35+
Name: "var",
36+
Usage: "Variable name to delete. Repeat for multiple names.",
37+
},
38+
),
39+
Action: func(ctx context.Context, cmd *cli.Command) error {
40+
if !cmd.Args().Present() {
41+
return cmderrors.Usage("must provide a workflow instance ID")
42+
}
43+
44+
instanceID := cmd.Args().First()
45+
client := cfg.apiClient(clicompat.APIFlagsFromCmd(cmd), cmd.Writer)
46+
variableNames := append(cmd.Args().Tail(), cmd.StringSlice("var")...)
47+
48+
return runABCVariablesDelete(ctx, client, instanceID, variableNames, cmd.Writer)
49+
},
50+
})
51+
52+
func runABCVariablesDelete(ctx context.Context, client api.Client, instanceID string, variableNames []string, output io.Writer) error {
53+
if len(variableNames) == 0 {
54+
return cmderrors.Usage("must provide at least one variable name")
55+
}
56+
57+
if slices.Contains(variableNames, "") {
58+
return cmderrors.Usage("variable names must not be empty")
59+
}
60+
61+
variables := make([]map[string]string, 0, len(variableNames))
62+
for _, key := range variableNames {
63+
variables = append(variables, map[string]string{
64+
"key": key,
65+
"value": "null",
66+
})
67+
}
68+
69+
ok, err := updateABCWorkflowInstanceVariables(ctx, client, instanceID, variables)
70+
if err != nil || !ok {
71+
return err
72+
}
73+
74+
_, err = fmt.Fprintf(output, "Removed variables %q from workflow instance %q.\n", variableNames, instanceID)
75+
return err
76+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"io"
7+
"testing"
8+
9+
mockapi "github.com/sourcegraph/src-cli/internal/api/mock"
10+
"github.com/stretchr/testify/mock"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestRunABCVariablesDelete(t *testing.T) {
15+
t.Parallel()
16+
17+
client := new(mockapi.Client)
18+
request := &mockapi.Request{Response: `{"data":{"updateAgenticWorkflowInstanceVariables":{"id":"workflow"}}}`}
19+
output := &bytes.Buffer{}
20+
variableNames := []string{"approval", "checkpoints", "prompt"}
21+
22+
client.On("NewRequest", updateABCWorkflowInstanceVariablesMutation, map[string]any{
23+
"instanceID": "QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ==",
24+
"variables": []map[string]string{
25+
{"key": "approval", "value": "null"},
26+
{"key": "checkpoints", "value": "null"},
27+
{"key": "prompt", "value": "null"},
28+
},
29+
}).Return(request).Once()
30+
request.On("Do", context.Background(), mock.Anything).Return(true, nil).Once()
31+
32+
err := runABCVariablesDelete(context.Background(), client, "QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ==", variableNames, output)
33+
require.NoError(t, err)
34+
require.Equal(t, "Removed variables [\"approval\" \"checkpoints\" \"prompt\"] from workflow instance \"QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ==\".\n", output.String())
35+
36+
client.AssertExpectations(t)
37+
request.AssertExpectations(t)
38+
}
39+
40+
func TestRunABCVariablesDeleteRejectsEmptyVariableName(t *testing.T) {
41+
t.Parallel()
42+
43+
err := runABCVariablesDelete(context.Background(), nil, "QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ==", []string{"approval", ""}, io.Discard)
44+
require.ErrorContains(t, err, "variable names must not be empty")
45+
}
46+
47+
func TestRunABCVariablesDeleteSuppressesSuccessMessageWhenRequestDoesNotExecute(t *testing.T) {
48+
t.Parallel()
49+
50+
client := new(mockapi.Client)
51+
request := &mockapi.Request{}
52+
output := &bytes.Buffer{}
53+
variableNames := []string{"approval", "checkpoints", "prompt"}
54+
55+
client.On("NewRequest", updateABCWorkflowInstanceVariablesMutation, map[string]any{
56+
"instanceID": "QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ==",
57+
"variables": []map[string]string{
58+
{"key": "approval", "value": "null"},
59+
{"key": "checkpoints", "value": "null"},
60+
{"key": "prompt", "value": "null"},
61+
},
62+
}).Return(request).Once()
63+
request.On("Do", context.Background(), mock.Anything).Return(false, nil).Once()
64+
65+
err := runABCVariablesDelete(context.Background(), client, "QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ==", variableNames, output)
66+
require.NoError(t, err)
67+
require.Empty(t, output.String())
68+
69+
client.AssertExpectations(t)
70+
request.AssertExpectations(t)
71+
}

cmd/src/abc_variables_set.go

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"encoding/json"
7+
"fmt"
8+
"io"
9+
"sort"
10+
"strings"
11+
12+
"github.com/sourcegraph/src-cli/internal/api"
13+
"github.com/sourcegraph/src-cli/internal/clicompat"
14+
"github.com/sourcegraph/src-cli/internal/cmderrors"
15+
"github.com/urfave/cli/v3"
16+
)
17+
18+
const updateABCWorkflowInstanceVariablesMutation = `mutation UpdateAgenticWorkflowInstanceVariables(
19+
$instanceID: ID!,
20+
$variables: [AgenticWorkflowInstanceVariableInput!]!,
21+
) {
22+
updateAgenticWorkflowInstanceVariables(instanceID: $instanceID, variables: $variables) {
23+
id
24+
}
25+
}`
26+
27+
var abcVariablesSetCommand = clicompat.Wrap(&cli.Command{
28+
Name: "set",
29+
UsageText: "src abc variables set [options] <workflow-instance-id> [<name>=<value> ...]",
30+
Usage: "Set variables on a workflow instance",
31+
DisableSliceFlagSeparator: true,
32+
Description: `
33+
Set workflow instance variables
34+
35+
Examples:
36+
37+
Set a string variable on a workflow instance:
38+
39+
$ src abc variables set QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ== prompt="tighten the review criteria"
40+
41+
Set multiple variables in one request:
42+
43+
$ src abc variables set QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ== --var prompt="tighten the review criteria" --var checkpoints='[1,2,3]'
44+
45+
Set a structured JSON value:
46+
47+
$ src abc variables set QWdlbnRpY1dvcmtmbG93SW5zdGFuY2U6MQ== checkpoints='[1,2,3]'
48+
49+
NOTE: Values are interpreted as JSON literals when valid. Otherwise they are sent as plain strings.
50+
`,
51+
Flags: clicompat.WithAPIFlags(
52+
&cli.StringSliceFlag{
53+
Name: "var",
54+
Usage: "Variable assignment in <name>=<value> form. Repeat to set multiple variables.",
55+
},
56+
),
57+
Action: func(ctx context.Context, cmd *cli.Command) error {
58+
if !cmd.Args().Present() {
59+
return cmderrors.Usage("must provide a workflow instance ID")
60+
}
61+
62+
instanceID := cmd.Args().First()
63+
client := cfg.apiClient(clicompat.APIFlagsFromCmd(cmd), cmd.Writer)
64+
abcVariables, err := parseABCVariables(cmd.Args().Tail(), cmd.StringSlice("var"))
65+
if err != nil {
66+
return err
67+
}
68+
return runABCVariablesSet(ctx, client, instanceID, abcVariables, cmd.Writer)
69+
},
70+
})
71+
72+
func parseABCVariables(positional []string, flagged []string) (map[string]string, error) {
73+
rawVariables := append(positional, flagged...)
74+
if len(rawVariables) == 0 {
75+
return nil, cmderrors.Usage("must provide at least one variable assignment")
76+
}
77+
78+
variables := make(map[string]string, len(rawVariables))
79+
for _, v := range rawVariables {
80+
name, rawValue, ok := strings.Cut(v, "=")
81+
if !ok || name == "" {
82+
return nil, cmderrors.Usagef("invalid variable assignment %q: must be in <name>=<value> form", v)
83+
}
84+
85+
value, remove, err := marshalABCVariableValue(rawValue)
86+
if err != nil {
87+
return nil, err
88+
}
89+
if remove {
90+
return nil, cmderrors.Usagef("invalid variable assignment %q: use 'src abc variables delete <workflow-instance-id> %s' to remove a variable", rawValue, name)
91+
}
92+
93+
variables[name] = value
94+
}
95+
96+
return variables, nil
97+
}
98+
99+
func runABCVariablesSet(ctx context.Context, client api.Client, instanceID string, variables map[string]string, output io.Writer) error {
100+
graphqlVariables := make([]map[string]string, 0, len(variables))
101+
keys := make([]string, 0, len(variables))
102+
for k := range variables {
103+
keys = append(keys, k)
104+
}
105+
sort.Strings(keys)
106+
107+
for _, k := range keys {
108+
graphqlVariables = append(graphqlVariables, map[string]string{
109+
"key": k,
110+
"value": variables[k],
111+
})
112+
}
113+
114+
ok, err := updateABCWorkflowInstanceVariables(ctx, client, instanceID, graphqlVariables)
115+
if err != nil || !ok {
116+
return err
117+
}
118+
119+
_, err = fmt.Fprintf(output, "Updated %d variables on workflow instance %q.\n", len(variables), instanceID)
120+
return err
121+
}
122+
123+
func updateABCWorkflowInstanceVariables(ctx context.Context, client api.Client, instanceID string, variables []map[string]string) (bool, error) {
124+
var result struct {
125+
UpdateAgenticWorkflowInstanceVariables struct {
126+
ID string `json:"id"`
127+
} `json:"updateAgenticWorkflowInstanceVariables"`
128+
}
129+
if ok, err := client.NewRequest(updateABCWorkflowInstanceVariablesMutation, map[string]any{
130+
"instanceID": instanceID,
131+
"variables": variables,
132+
}).Do(ctx, &result); err != nil || !ok {
133+
return ok, err
134+
}
135+
136+
return true, nil
137+
}
138+
139+
func marshalABCVariableValue(raw string) (value string, remove bool, err error) {
140+
// Try to compact valid JSON literals first so numbers, arrays, and objects are sent unchanged.
141+
// A bare null is detected separately so the CLI can require the explicit delete command.
142+
// If compacting doesn't work for the given value, fall back to string encoding.
143+
var compact bytes.Buffer
144+
if err := json.Compact(&compact, []byte(raw)); err == nil {
145+
value := compact.String()
146+
return value, value == "null", nil
147+
}
148+
149+
encoded, err := json.Marshal(raw)
150+
if err != nil {
151+
return "", false, err
152+
}
153+
154+
return string(encoded), false, nil
155+
}

0 commit comments

Comments
 (0)