Skip to content

Commit a02b65f

Browse files
authored
Comment commands address the adopter's own App (#131)
* Comment commands address the adopter's own App App names are globally unique, so every installation has a different bot login. A constant handle could never be right in more than one repository: ours matched nothing in anyone else's, and because our App is public it rendered there as a mention of an account they never installed. /simplycubed avoided that but gave up the thing that makes a handle worth having. GitHub offers accounts with repository access in the autocomplete, so a real mention completes after someone types @A, without them knowing the bot's name. A prefix that is not an account cannot, and typing / opens GitHub's own menu, which then matches nothing. appName in .github/simplycubed.yml is the source of truth. The parser, the help text, and the wrong-surface replies all render from it, so the agent never tells anyone to mention a different account. The workflow trigger stays a literal, written by init from the same value. A workflow decides whether to start before any code runs, so matching loosely there would spin up a runner every time someone mentions a colleague: token, checkout, Go, CLI, sandbox, roughly thirty seconds, to then do nothing. That means the handle lives in two files and the App can only push one of them. preflight fails when they disagree, because the alternative is silent: comments stop working with no error anywhere. * Cover the paths this change introduced Codecov flagged 10 uncovered lines across main.go and command.go. Every one was a branch the change added and nothing exercised. resolveAppName decides what handle init writes, so each way it can be reached is now pinned: the flag, an existing config, the flag overriding a config, and neither. That last one matters most, because the error has to explain that the name is the adopter's rather than just demand a flag. appNameFor parses --repo-dir out of flags it does not own; both spellings are covered, since reading the wrong repository's config would silently answer as the wrong bot. Addressed and Parse with no configured App: a repository that has not said which App it installed has no handle to answer to. commandCmd surfacing a config error rather than parsing nothing, which would look like a comment the agent chose to ignore.
1 parent 0d99484 commit a02b65f

12 files changed

Lines changed: 490 additions & 146 deletions

File tree

‎README.md‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,19 @@ You drive the agent by applying one label and reading the pull request. The bot
9696

9797
You can also drive it by comment, addressed to the bot at the start of a line:
9898

99-
- `/simplycubed go` on an issue starts work on it, the same as applying `sc:go`.
100-
- `/simplycubed address` on a pull request addresses the current review feedback.
101-
- `/simplycubed help` lists what it understands.
99+
Commands address **your own App**, not ours. If you installed an App called
100+
`acme-code`, your team types:
101+
102+
- `@acme-code go` on an issue starts work on it, the same as applying `sc:go`.
103+
- `@acme-code address` on a pull request addresses the current review feedback.
104+
- `@acme-code help` lists what it understands.
105+
106+
The handle is per-repository because App names are globally unique, so every
107+
installation has its own. It is also why GitHub offers the bot in the
108+
autocomplete after someone types `@`: it suggests accounts with access to the
109+
repository, which a fixed prefix could never be. You set it once with
110+
`simplycubed init --app-name`, and it is stored as `appName:` in
111+
`.github/simplycubed.yml`.
102112

103113
Only comments from people with write access are acted on, and only a comment that begins with the mention counts, so quoting an earlier comment never re-triggers a run. Note that a plain pull-request comment is not a review: to run the fixer from a review, submit it through **Files changed → Review changes**.
104114

‎STATUS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ internal/forge/ GitHub side as an interface (no merge method); gh/ adapter
6464
internal/vcs/git/ commit, push, and sync-to-PR-head
6565
internal/describe/ generated PR walkthrough, changes table, and sequence diagram
6666
internal/verdict/ the reviewer verdict: schema, validation, findings for the fixer
67-
internal/command/ comment commands (/simplycubed go | address | help)
67+
internal/command/ comment commands (@<your-app> go | address | help)
6868
internal/forge/dryrun/ records the GitHub writes a dry run skips
6969
internal/attribution/ the SimplyCubed Code marker on generated commits and PRs
7070
internal/ledger/ append-only JSONL run events

‎cmd/simplycubed/main.go‎

Lines changed: 111 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,18 @@ const starterConfig = `# Repository-local contract for SimplyCubed Code.
124124
# Keep the label prefix unless you already run another sc:* lifecycle.
125125
labelPrefix: sc
126126
127+
# The GitHub App you created and installed, without the "[bot]" suffix. Comment
128+
# commands address it: "@__SIMPLYCUBED_APP_NAME__ go" on an issue starts work.
129+
#
130+
# This is yours, not ours. App names are globally unique, so every installation
131+
# has a different one, and addressing your real bot is what makes GitHub offer
132+
# it in the autocomplete after someone types "@".
133+
#
134+
# The caller workflow triggers on this same handle. If you rename the App,
135+
# change it here and re-run "simplycubed init --workflow"; preflight fails if
136+
# the two ever disagree.
137+
appName: __SIMPLYCUBED_APP_NAME__
138+
127139
# Required. Fill this in with the real gate that is already green on main.
128140
gate:
129141
@@ -138,8 +150,9 @@ gate:
138150
`
139151

140152
const (
141-
latestKnownWorkflowTag = "v0.2.0"
142-
callerWorkflowTagToken = "__SIMPLYCUBED_TAG__"
153+
latestKnownWorkflowTag = "v0.2.0"
154+
callerWorkflowTagToken = "__SIMPLYCUBED_TAG__"
155+
callerWorkflowAppNameToken = "__SIMPLYCUBED_APP_NAME__"
143156
)
144157

145158
//go:embed simplycubed-caller.yml.tmpl
@@ -266,6 +279,29 @@ func takeBody(argv []string) (body string, rest []string, err error) {
266279
return body, rest, nil
267280
}
268281

282+
// appNameFor reads the configured App handle from the same --repo-dir the rest
283+
// of the command will use. It parses only that flag, because run and address
284+
// own the full flag set and a second copy of it would drift.
285+
func appNameFor(argv []string) (string, error) {
286+
repoDir := "."
287+
for i, a := range argv {
288+
switch {
289+
case a == "--repo-dir" || a == "-repo-dir":
290+
if i+1 < len(argv) {
291+
repoDir = argv[i+1]
292+
}
293+
case strings.HasPrefix(a, "--repo-dir="), strings.HasPrefix(a, "-repo-dir="):
294+
_, v, _ := strings.Cut(a, "=")
295+
repoDir = v
296+
}
297+
}
298+
cfg, err := config.Load(filepath.Join(repoDir, ".github", "simplycubed.yml"))
299+
if err != nil {
300+
return "", fmt.Errorf("load config: %w", err)
301+
}
302+
return cfg.AppName, nil
303+
}
304+
269305
// commandCmd routes a comment addressed to the agent to the matching loop. The
270306
// comment body is untrusted, so it is parsed into a fixed vocabulary here and
271307
// never interpreted: an unrecognised comment does nothing at all.
@@ -277,11 +313,18 @@ func commandCmd(argv []string, stdout io.Writer) error {
277313
if err != nil {
278314
return err
279315
}
280-
kind := command.Parse(body)
316+
// The handle is per-repository, so the config has to be read before the
317+
// comment can be parsed at all: which App this repository installed is the
318+
// thing that decides what a command even looks like here.
319+
appName, err := appNameFor(rest)
320+
if err != nil {
321+
return err
322+
}
323+
kind := command.Parse(body, appName)
281324

282325
switch kind {
283326
case command.Help:
284-
return reply(rest, command.HelpText, stdout)
327+
return reply(rest, command.HelpTextFor(appName), stdout)
285328
case command.Go, command.Address:
286329
// A verb aimed at the wrong surface is answered, not run. This lives on
287330
// the comment path rather than inside run and address because someone
@@ -292,7 +335,7 @@ func commandCmd(argv []string, stdout io.Writer) error {
292335
return err
293336
}
294337
if a.ok && command.Misdirected(kind, a.onPR) {
295-
return a.post(command.MisdirectedText(kind, a.onPR), stdout)
338+
return a.post(command.MisdirectedText(kind, a.onPR, appName), stdout)
296339
}
297340
if kind == command.Go {
298341
return runCmd(rest)
@@ -302,8 +345,8 @@ func commandCmd(argv []string, stdout io.Writer) error {
302345
// Only answer a comment that was actually addressed to the agent.
303346
// Anything else is someone mentioning it in passing, and replying to
304347
// that would make it the noisiest participant in every thread.
305-
if command.Addressed(body) {
306-
return reply(rest, command.UnknownText, stdout)
348+
if command.Addressed(body, appName) {
349+
return reply(rest, command.UnknownTextFor(appName), stdout)
307350
}
308351
fmt.Fprintln(stdout, "no command recognised in that comment; nothing to do")
309352
return nil
@@ -407,6 +450,29 @@ func reply(argv []string, body string, stdout io.Writer) error {
407450
// preflightCmd validates the repo config and engine settings, then exits. A
408451
// workflow runs it before installing the rest of the toolchain, so a
409452
// misconfigured repository finds out in seconds and the rules live in one place.
453+
// checkMentionAgreement fails when the configured handle and the caller
454+
// workflow's trigger disagree.
455+
//
456+
// The handle necessarily lives in two files: the parser reads it from config,
457+
// which the App can push, while the trigger has to be a literal in the workflow,
458+
// which the App deliberately cannot. Drift between them is silent in the worst
459+
// way — comments simply stop working, with no error anywhere — so it is checked
460+
// on every run rather than left to be discovered.
461+
func checkMentionAgreement(repoDir, appName string) error {
462+
path := filepath.Join(repoDir, ".github", "workflows", "simplycubed.yml")
463+
b, err := os.ReadFile(path)
464+
if err != nil {
465+
// No caller workflow is a valid local setup, not a misconfiguration.
466+
return nil
467+
}
468+
want := "'" + command.MentionFor(appName) + "'"
469+
if strings.Contains(string(b), want) {
470+
return nil
471+
}
472+
return fmt.Errorf("%w: .github/simplycubed.yml sets appName %q, but %s does not trigger on %s. Comment commands will never fire. Re-run \"simplycubed init --workflow\" to rewrite the trigger from the config",
473+
ErrConfigMissing, appName, path, want)
474+
}
475+
410476
func preflightCmd(argv []string, stdout io.Writer) error {
411477
fs := flag.NewFlagSet("preflight", flag.ContinueOnError)
412478
repoDir := fs.String("repo-dir", ".", "path to the target repo checkout")
@@ -421,6 +487,12 @@ func preflightCmd(argv []string, stdout io.Writer) error {
421487
if _, err := engineEnv(cfg); err != nil {
422488
return err
423489
}
490+
if cfg.AppName == "" {
491+
return fmt.Errorf("%w: .github/simplycubed.yml sets no appName. Comment commands address the App you installed, so without it none of them can fire. Add \"appName: <your-app>\" or re-run \"simplycubed init --workflow --app-name <your-app>\"", ErrConfigMissing)
492+
}
493+
if err := checkMentionAgreement(*repoDir, cfg.AppName); err != nil {
494+
return err
495+
}
424496
// The App credentials are only reachable when the caller workflow exports
425497
// them. A local run authenticates as the operator and never uses them, so
426498
// checking them there would fail a working setup.
@@ -651,12 +723,18 @@ func initCmd(argv []string, stdout io.Writer) error {
651723
fs := flag.NewFlagSet("init", flag.ContinueOnError)
652724
repoDir := fs.String("repo-dir", ".", "path to the target repo checkout")
653725
writeWorkflow := fs.Bool("workflow", false, "also write the Actions caller workflow")
726+
appNameFlag := fs.String("app-name", "", "the GitHub App you installed, for example acme-code; comment commands address it")
654727
if _, err := parseInterleaved(fs, argv); err != nil {
655728
return err
656729
}
657730

731+
appName, err := resolveAppName(*repoDir, *appNameFlag)
732+
if err != nil {
733+
return err
734+
}
735+
658736
configPath := filepath.Join(*repoDir, ".github", "simplycubed.yml")
659-
wroteConfig, err := writeStarterConfig(configPath)
737+
wroteConfig, err := writeStarterConfig(configPath, appName)
660738
if err != nil {
661739
return err
662740
}
@@ -665,7 +743,7 @@ func initCmd(argv []string, stdout io.Writer) error {
665743
wroteWorkflow := false
666744
wroteSelftest := false
667745
if *writeWorkflow {
668-
wroteWorkflow, err = writeStarterFile(workflowPath, renderCallerWorkflow(buildinfo.Version))
746+
wroteWorkflow, err = writeStarterFile(workflowPath, renderCallerWorkflow(buildinfo.Version, appName))
669747
if err != nil {
670748
return err
671749
}
@@ -771,8 +849,22 @@ func appCreateURL(repoDir string) (string, bool) {
771849
return "https://github.com/settings/apps/new", true
772850
}
773851

774-
func writeStarterConfig(path string) (bool, error) {
775-
return writeStarterFile(path, starterConfig)
852+
func writeStarterConfig(path, appName string) (bool, error) {
853+
return writeStarterFile(path, strings.ReplaceAll(starterConfig, callerWorkflowAppNameToken, appName))
854+
}
855+
856+
// resolveAppName decides the handle to write into both files. An explicit
857+
// --app-name wins; otherwise an existing config keeps what it already says, so
858+
// re-running init to pick up a new release does not silently change the handle
859+
// a team already types.
860+
func resolveAppName(repoDir, flagValue string) (string, error) {
861+
if v := strings.TrimSuffix(strings.TrimPrefix(strings.TrimSpace(flagValue), "@"), "[bot]"); v != "" {
862+
return v, nil
863+
}
864+
if cfg, err := config.Load(filepath.Join(repoDir, ".github", "simplycubed.yml")); err == nil && cfg.AppName != "" {
865+
return cfg.AppName, nil
866+
}
867+
return "", errors.New("--app-name is required: comment commands address the App you installed, and its name is unique to you. Pass the App's name without the \"[bot]\" suffix, for example --app-name acme-code")
776868
}
777869

778870
func writeStarterFile(path, body string) (bool, error) {
@@ -790,8 +882,14 @@ func writeStarterFile(path, body string) (bool, error) {
790882
return true, nil
791883
}
792884

793-
func renderCallerWorkflow(version string) string {
794-
return strings.ReplaceAll(callerWorkflowTemplate, callerWorkflowTagToken, workflowTemplateTag(version))
885+
// renderCallerWorkflow writes the trigger for the App this repository installed.
886+
// The handle cannot be a constant: App names are globally unique, so every
887+
// adopter's bot has its own, and the trigger has to match theirs or no comment
888+
// ever starts a run. It is templated here rather than matched loosely at
889+
// runtime so a mention of a colleague does not spin up a runner.
890+
func renderCallerWorkflow(version, appName string) string {
891+
out := strings.ReplaceAll(callerWorkflowTemplate, callerWorkflowTagToken, workflowTemplateTag(version))
892+
return strings.ReplaceAll(out, callerWorkflowAppNameToken, appName)
795893
}
796894

797895
func workflowTemplateTag(version string) string {

0 commit comments

Comments
 (0)