Commit `dfc4fecaa3`

dfc4fecaa30f20d76db2eface9555552712f8333

parent: 541f0d571c

Verified · cmc ci/build: success

cmc <hello@cleberg.net> · 2026-08-30T18:09:33Z

Resolve builds a dead runner abandoned, and forward bare-redirect stdin

A runner killed between claiming a build and reporting it left the row running
forever, and the commit's ci/<job> status pending with it. `runner next` now
reaps builds past a 90m deadline — comfortably longer than the runner's own 45m
timeout, so it only fires when nothing reported at all — failing them with a log
line and resolving their commit status.

`keys add` and `repo deploy-key add` take stdin as a bare `< key.pub`, but were
wired stdinOK, which only forwards stdin when `--file -` appears in the
arguments. Both reached the server with an empty body and failed on input the
SSH API accepts. A coverage test now pins the four commands whose payload is an
unnamed redirect.

Closes #53
Closes #55

cmd/gitbay/coverage_test.go +36

		@@ -55,3 +55,39 @@ func TestEveryCommandIsReachable(t *testing.T) {
55	55	}
56	56	}
57	57	}
	58
	59	// Commands whose payload is stdin with no flag naming it: `gitbay keys add <
	60	// key.pub`, not `--file -`. They must be wired alwaysStdin, because the
	61	// stdinOK path only forwards stdin when it sees `--file -` in the arguments —
	62	// so a bare redirect reached the server as an empty body and the command
	63	// failed on input the SSH API accepts.
	64	var bareStdin = []string{
	65	"keys add",
	66	"repo deploy-key add",
	67	"repo secret set",
	68	"release asset add",
	69	}
	70
	71	func TestBareRedirectCommandsAlwaysForwardStdin(t *testing.T) {
	72	modes := map[string]string{}
	73	var walk func(*cobra.Command)
	74	walk = func(c *cobra.Command) {
	75	if p := c.Annotations[serverPath]; p != "" {
	76	modes[p] = c.Annotations[stdinMode]
	77	}
	78	for _, sub := range c.Commands() {
	79	walk(sub)
	80	}
	81	}
	82	walk(newRoot())
	83
	84	for _, path := range bareStdin {
	85	switch modes[path] {
	86	case "always":
	87	case "":
	88	t.Errorf("%q is not wired in the CLI", path)
	89	default:
	90	t.Errorf("%q is wired %s; a bare redirect needs alwaysStdin", path, modes[path])
	91	}
	92	}
	93	}

cmd/gitbay/main.go +24 −7

		@@ -83,23 +83,40 @@ func newRoot() *cobra.Command {
83	83	// serverPath is the annotation key holding a passthrough command's
84	84	// server-side path, so the tree can be checked against the registry.
85	85	const serverPath = "gitbay.server_path"
	86	const stdinMode = "gitbay.stdin_mode"
86	87
87	88	// passOpts describes how one CLI command maps onto the server command.
88	89	type passOpts struct {
89	90	server []string // server-side command path
90	91	needsRepo bool // prepend inferred owner/name unless given
91		stdinOK bool // wire local stdin through (keys add, --file -)
92		alwaysStdin bool // stdin is the payload (release asset add)
	92	stdinOK bool // wire local stdin through when --file - asks for it
	93	alwaysStdin bool // stdin is the payload, named by no flag: a bare redirect
93	94	editor string // open $EDITOR for a body when none given
94	95	}
95	96
96	97	// pass builds a passthrough command. Flags are parsed by the server, which
97	98	// is the single source of truth for them; the CLI stays thin.
	99	// stdinModeName reports how this command takes stdin, so the coverage test can
	100	// check that a command reading a bare redirect is not left waiting for a
	101	// `--file -` that its callers never type.
	102	func (o passOpts) stdinModeName() string {
	103	switch {
	104	case o.alwaysStdin:
	105	return "always"
	106	case o.stdinOK:
	107	return "flag"
	108	}
	109	return "none"
	110	}
	111
98	112	func pass(use, short string, o passOpts) *cobra.Command {
99	113	return &cobra.Command{
100		Use: use,
101		Short: short,
102		Annotations: map[string]string{serverPath: strings.Join(o.server, " ")},
	114	Use: use,
	115	Short: short,
	116	Annotations: map[string]string{
	117	serverPath: strings.Join(o.server, " "),
	118	stdinMode: o.stdinModeName(),
	119	},
103	120	DisableFlagParsing: true,
104	121	RunE: func(cmd *cobra.Command, args []string) error {
105	122	// cobra still owns `forge <cmd> --help`.
		@@ -201,7 +218,7 @@ func local(use, short string, fn func(args []string) int) *cobra.Command {
201	218
202	219	func authCmd() *cobra.Command {
203	220	keysAdd := pass("add", "register an SSH public key (reads the key from stdin or --file -)",
204		passOpts{server: []string{"keys", "add"}, stdinOK: true})
	221	passOpts{server: []string{"keys", "add"}, alwaysStdin: true})
205	222	// keys add always reads stdin on the server; wire it through directly.
206	223	keysAdd.RunE = func(cmd *cobra.Command, args []string) error {
207	224	t, err := resolveTarget()
		@@ -283,7 +300,7 @@ func repoCmd() *cobra.Command {
283	300	pass("import-issues", "import GitHub issue/PR history: --from <ghowner/ghrepo> [--token-stdin]",
284	301	passOpts{server: []string{"repo", "import-issues"}, needsRepo: true, stdinOK: true}),
285	302	group("deploy-key", "repository-bound CI keys",
286		pass("add", "bind a key: [--rw] < key.pub", passOpts{server: []string{"repo", "deploy-key", "add"}, needsRepo: true, stdinOK: true}),
	303	pass("add", "bind a key: [--rw] < key.pub", passOpts{server: []string{"repo", "deploy-key", "add"}, needsRepo: true, alwaysStdin: true}),
287	304	pass("list", "list deploy keys", passOpts{server: []string{"repo", "deploy-key", "list"}, needsRepo: true}),
288	305	pass("remove", "remove a deploy key: <fingerprint>", passOpts{server: []string{"repo", "deploy-key", "remove"}, needsRepo: true}),
289	306	),

internal/control/build.go +15

		@@ -301,6 +301,21 @@ func runRunnerNext(c *Ctx, args []string) int {
301	301	if code := requireRunner(c); code >= 0 {
302	302	return code
303	303	}
	304	// Resolve anything a previous runner claimed and never reported, so a
	305	// killed runner does not leave a build running and a commit pending forever.
	306	if stale, err := c.Store.ReapStaleBuilds(); err != nil {
	307	return c.fail(protocol.ExitFailure, "%v", err)
	308	} else {
	309	for _, sb := range stale {
	310	repo, err := c.Store.RepoByID(sb.RepoID)
	311	if err != nil {
	312	continue
	313	}
	314	url := fmt.Sprintf("%s/%s/builds/%d", c.Cfg.Server.SiteURL, repo.Path(), sb.Number)
	315	c.Store.SetCommitStatus(repo.ID, sb.SHA, "ci/"+sb.Job, "failure",
	316	"build abandoned", url, c.User.ID)
	317	}
	318	}
304	319	b, ok, err := c.Store.ClaimBuild()
305	320	if err != nil {
306	321	return c.fail(protocol.ExitFailure, "%v", err)

internal/store/builds.go +42

		@@ -3,6 +3,7 @@ package store
3	3	import (
4	4	"database/sql"
5	5	"errors"
	6	"time"
6	7	)
7	8
8	9	// Build is one CI job execution for one commit.
		@@ -83,6 +84,47 @@ func (s *Store) ClaimBuild() (Build, bool, error) {
83	84	return b, true, tx.Commit()
84	85	}
85	86
	87	// StaleBuildDeadline is how long a claimed build may stay running before the
	88	// server gives up on it. Comfortably longer than the runner's own -timeout
	89	// (45m by default), so this only fires when the runner never reported at all —
	90	// it was killed, restarted, or lost the network mid-build.
	91	const StaleBuildDeadline = 90 * time.Minute
	92
	93	// ReapStaleBuilds fails every build that has been running past the deadline and
	94	// returns them, so the caller can resolve their commit statuses. A runner that
	95	// dies between claiming a build and reporting it otherwise leaves the row
	96	// claimed forever, and the commit pending forever with it.
	97	func (s *Store) ReapStaleBuilds() ([]Build, error) {
	98	cutoff := time.Now().UTC().Add(-StaleBuildDeadline).Format("2006-01-02T15:04:05Z")
	99	rows, err := s.DB.Query(buildSelect+
	100	" WHERE status = 'running' AND started_at != '' AND started_at < ?", cutoff)
	101	if err != nil {
	102	return nil, err
	103	}
	104	defer rows.Close()
	105	var stale []Build
	106	for rows.Next() {
	107	b, err := scanBuild(rows)
	108	if err != nil {
	109	return nil, err
	110	}
	111	stale = append(stale, b)
	112	}
	113	if err := rows.Err(); err != nil {
	114	return nil, err
	115	}
	116	for _, b := range stale {
	117	if err := s.AppendBuildLog(b.ID, []byte(
	118	"\nbuild abandoned: the runner never reported an outcome\n")); err != nil {
	119	return nil, err
	120	}
	121	if err := s.FinishBuild(b.ID, "failure"); err != nil {
	122	return nil, err
	123	}
	124	}
	125	return stale, nil
	126	}
	127
86	128	// AppendBuildLog adds a chunk to the build's log, dropping bytes past the cap.
87	129	func (s *Store) AppendBuildLog(id int64, chunk []byte) error {
88	130	_, err := s.DB.Exec(`

internal/store/builds_test.go added +71

		@@ -0,0 +1,71 @@
	1	package store
	2
	3	import (
	4	"strings"
	5	"testing"
	6	)
	7
	8	// A runner that dies between claiming a build and reporting it leaves the row
	9	// claimed. The next claim resolves it rather than leaving the build running and
	10	// the commit pending forever.
	11	func TestReapStaleBuilds(t *testing.T) {
	12	s := open(t)
	13	if err := s.MigrateUp(); err != nil {
	14	t.Fatal(err)
	15	}
	16	uid, err := s.CreateUser("cmc", true)
	17	if err != nil {
	18	t.Fatal(err)
	19	}
	20	if _, err := s.CreateRepo("user", uid, "orgo", "public"); err != nil {
	21	t.Fatal(err)
	22	}
	23
	24	stuck, err := s.CreateBuild(1, "test", "abc123", "main", `["true"]`)
	25	if err != nil {
	26	t.Fatal(err)
	27	}
	28	fresh, err := s.CreateBuild(1, "pages", "abc123", "main", `["true"]`)
	29	if err != nil {
	30	t.Fatal(err)
	31	}
	32
	33	// Claim both, then age only the first past the deadline.
	34	for range 2 {
	35	if _, ok, err := s.ClaimBuild(); err != nil \|\| !ok {
	36	t.Fatalf("claim: %v ok=%v", err, ok)
	37	}
	38	}
	39	if _, err := s.DB.Exec(
	40	`UPDATE builds SET started_at = '2020-01-01T00:00:00Z' WHERE number = ?`, stuck); err != nil {
	41	t.Fatal(err)
	42	}
	43
	44	reaped, err := s.ReapStaleBuilds()
	45	if err != nil {
	46	t.Fatal(err)
	47	}
	48	if len(reaped) != 1 \|\| reaped[0].Number != stuck {
	49	t.Fatalf("reaped %+v, want only build %d", reaped, stuck)
	50	}
	51
	52	b, err := s.BuildByNumber(1, stuck)
	53	if err != nil {
	54	t.Fatal(err)
	55	}
	56	if b.Status != "failure" \|\| b.FinishedAt == "" {
	57	t.Fatalf("stale build is %s finished %q, want failure with a timestamp", b.Status, b.FinishedAt)
	58	}
	59	log, err := s.BuildLog(b.ID)
	60	if err != nil {
	61	t.Fatal(err)
	62	}
	63	if !strings.Contains(string(log), "abandoned") {
	64	t.Fatalf("log does not say why it failed: %q", log)
	65	}
	66
	67	// A build still inside the deadline is left alone.
	68	if b, err := s.BuildByNumber(1, fresh); err != nil \|\| b.Status != "running" {
	69	t.Fatalf("fresh build is %v (%v), want running", b.Status, err)
	70	}
	71	}

Commit dfc4fecaa3

Commit `dfc4fecaa3`