Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/changes/unreleased/1801-senior-dev-ending-no-checks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
kind: fixed
title: senior-dev's ending no longer says the build and tests passed when it ran none
pr: 1801
surface: [engine, docs]
invalidates:
- "senior-dev ended `submitted a change, and the project's own build and tests passed` whenever its check came back clean, even in a folder with nothing to run. With zero commands it now ends `submitted a change; the project has no build or tests it could find to run`."
- "The run's reason for a zero-command pass, which codeaf appends to the landing note, said `submitted, and its build and tests passed`. It now says `submitted, and found no build or tests to run`."
---

A real run on a folder holding only a README printed the pass sentence one line
above `senior-dev observed: the project has no build or tests it could find to run`.
The inner status is still `pass`; only what the ending claims changed.
34 changes: 31 additions & 3 deletions internal/manual/chat/senior-dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ When it submits, its submission receipt names the change, for example
`across 3 file(s), tree <id>`. In a plain folder it counts changed files but does not
produce patch text, so no byte size is shown; a measured patch in a git repository says,
for example, `128 bytes across 3 file(s), tree <id>`. The run's last line is its ending
sentence, such as that it submitted a change and the project's own build and tests passed.
sentence, such as `submitted a change, and the project's own build and tests passed`,
which it says only when at least one of their commands ran.

**On Windows it is absent**: there is no `/senior-dev` and no `codeaf senior-dev`. Its
engine needs a Unix shell, process groups and file locks, so Windows builds leave it out
Expand Down Expand Up @@ -501,6 +502,32 @@ have found. senior-dev runs them itself on the submitted tree, with the same str
settings and time limit, and a non-zero exit fails the check. Naming one never
excuses the other. Only the command line sets them; senior-dev's own model cannot.

## senior-dev found no build or tests to run — a folder with only a README, nothing was checked, did it pass

A folder that does not look like a project — no manifest, no `Makefile`, no
`CMakeLists.txt`, no `test/` folder, only a README or loose files — has no build or
tests for senior-dev to find, so its own check runs no command. Its `verify` step says
`found no build or tests to run`.

A change it submits there is still finished work, and its ending says only what was
checked:
`finished: submitted a change; the project has no build or tests it could find to run`.
At a shell, the observation printed beneath it says the same:
`senior-dev observed: the project has no build or tests it could find to run`.
It never says the project's build and tests passed when none ran; that sentence,
`submitted a change, and the project's own build and tests passed`, is kept for a run
where at least one of their commands did.

To the chat the run still counts as passed (see what codeaf does when senior-dev ends):
it reads the change and offers to merge the branch without any check of its own having
run, so look at the change yourself before taking it.

A folder that does look like a project but has no command senior-dev can find is
different: its check fails with `no build entrypoint could be discovered` (or `test`),
and the change is still handed in (see how senior-dev finds a project's build and
tests). To have a folder checked by something it cannot find, name the check yourself
with `--verify-test` or `--verify-build` on `codeaf senior-dev run`.

## Can I run senior-dev in a folder that is not a git repo — a plain folder, no git, --in-place, operation not permitted, .Trash

Yes. **senior-dev uses git only if it is there.** A folder with no git history — a plain
Expand Down Expand Up @@ -1204,8 +1231,9 @@ senior-dev had finished but that codeaf closed under before the run was over rea
A run ends in one of these ways, and the task's ending says which:

- `finished: …` — it submitted a change, and the words after say what its own check of the
project's build and tests found on the frozen tree, passed or not: a change handed in is
finished work, and a check that did not pass is looked into by the chat, not acted on;
project's build and tests found on the frozen tree, passed or not, or that it found none
to run: a change handed in is finished work, and a check that did not pass is looked
into by the chat, not acted on;
- `senior-dev did not finish: …` — it ended without submitting. It is not drawn as a fault,
what it made is still on its branch, and the chat acts on it (see what codeaf does when
senior-dev ends);
Expand Down
24 changes: 24 additions & 0 deletions internal/manual/chat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3214,6 +3214,30 @@ func TestGeneralTaskCostPagesNameSeniorDevCeiling(t *testing.T) {
}
}

// A senior-dev run in a folder with nothing to check ends saying so, where it
// once said the project's build and tests passed. Whoever reads that ending, or
// doubts an older one, asks in these words and must reach the section that says
// what was checked.
func TestSeniorDevNothingToRunQuestionsReachItsSection(t *testing.T) {
for _, asked := range []string{
"senior-dev says the project has no build or tests it could find to run",
"senior-dev found no build or tests to run",
"my folder only has a README, did senior-dev check its change",
"senior-dev said the build and tests passed but my folder has no tests",
} {
found := false
for _, section := range Chat().Search(asked, DefaultResults) {
if section.Page == "senior-dev" && strings.Contains(section.Title, "found no build or tests to run") {
found = true
break
}
}
if !found {
t.Errorf("%q does not reach the section on a folder with nothing to run", asked)
}
}
}

// TestC13UpdateQuestionsReachTheNewManualSection proves C13.
func TestC13UpdateQuestionsReachTheNewManualSection(t *testing.T) {
for _, asked := range []string{
Expand Down
101 changes: 101 additions & 0 deletions internal/seniordev/app/ending_message_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
//go:build !windows

package app

import (
"context"
"io"
"strings"
"testing"

"github.com/Agent-Field/codeaf/internal/delegate"
)

// A pass is said as passed only when the project's own build and tests ran.
// A real run on a folder holding only a README printed
//
// senior-dev finished: submitted a change, and the project's own build and tests passed
// senior-dev observed: the project has no build or tests it could find to run
//
// two witnesses contradicting each other about one check, because the ending's
// sentence read the status and never the count of commands behind it.
func TestPassEndingSaysOnlyWhatWasChecked(t *testing.T) {
const passed = "submitted a change, and the project's own build and tests passed"
for _, test := range []struct {
name string
data map[string]any
message string
observed string
}{
{
name: "commands ran",
data: map[string]any{"status": "pass", "verification_commands": 3, "verification_failing": 0},
message: passed,
observed: "the project's 3 build and test commands all passed",
},
{
// The same record after it has crossed JSON, which is how codeaf
// holds it once the run has ended.
name: "commands ran, read back from JSON",
data: map[string]any{"status": "pass", "verification_commands": float64(2), "verification_failing": float64(0)},
message: passed,
observed: "the project's 2 build and test commands all passed",
},
{
name: "no command ran",
data: map[string]any{"status": "pass", "verification_commands": 0, "verification_failing": 0},
message: "submitted a change; the project has no build or tests it could find to run",
observed: "the project has no build or tests it could find to run",
},
} {
t.Run(test.name, func(t *testing.T) {
ending := endingOf(pipelineResult{Status: delegate.StatusPass, Terminal: test.data})
if ending.Message != test.message {
t.Errorf("message = %q, want %q", ending.Message, test.message)
}
if ending.Observed != test.observed {
t.Errorf("observed = %q, want %q", ending.Observed, test.observed)
}
})
}
}

// The same ending from the real road: a submitted change in a folder with
// nothing discoverable to run, through the project's own check and the
// terminal it writes. Neither the sentence nor the longer reason beside it may
// say anything passed.
func TestFolderWithNothingToRunEndsWithoutSayingItPassed(t *testing.T) {
workspace := gitWorkspace(t, map[string]string{"README.md": "Notes about this folder.\n"})
base := strings.TrimSpace(gitOutput(context.Background(), workspace, "rev-parse", "HEAD"))
runner := newPipeline(cliArgs{}, workspace, pipelineDeps{
Backend: &soloScriptedBackend{}, Events: newEventWriter(io.Discard), Notes: io.Discard,
})
defer runner.runtime.Close()

outcome, err := runner.runSolo(context.Background(), "Add the feature.", base)
if err != nil {
t.Fatal(err)
}
status, reason := soloResultStatus(outcome)
if status != delegate.StatusPass || outcome.Status != "pass" {
t.Fatalf("status = %q (inner %q), want a pass: %#v", status, outcome.Status, outcome.TerminalData)
}
if commands, ok := outcome.TerminalData["verification_commands"]; !ok || wholeNumber(commands) != 0 {
t.Fatalf("verification_commands = %#v, want 0 for a folder with nothing to run", commands)
}
ending := endingOf(pipelineResult{Status: status, Reason: reason, Terminal: outcome.TerminalData})
if want := "submitted a change; the project has no build or tests it could find to run"; ending.Message != want {
t.Errorf("message = %q, want %q", ending.Message, want)
}
if want := "the project has no build or tests it could find to run"; ending.Observed != want {
t.Errorf("observed = %q, want %q", ending.Observed, want)
}
for name, said := range map[string]string{"message": ending.Message, "reason": ending.Reason} {
if strings.Contains(said, "passed") {
t.Errorf("%s = %q, which says something passed when no command ran", name, said)
}
}
if !strings.Contains(ending.Reason, "found no build or tests to run") {
t.Errorf("reason = %q, want it to say no build or tests were found to run", ending.Reason)
}
}
16 changes: 15 additions & 1 deletion internal/seniordev/app/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,27 @@ func endingOf(result pipelineResult) delegate.Ending {
return ending
}

// nothingToCheck is what senior-dev saw of a project whose build and tests it
// could not find, so ran none. The ending's sentence and its observation both
// say it, in these words.
const nothingToCheck = "the project has no build or tests it could find to run"

// messageOf is the ending in one sentence. A run that submitted is said in
// terms of what its own check of the project found, which is the fact the
// status projects; everything else keeps the reason the run gave.
//
// A PASS WITH NO COMMAND BEHIND IT PASSED NOTHING. A folder that does not look
// like a project — a README and nothing else — has no build or tests to find,
// and its check comes back clean having run zero commands. Saying its build
// and tests passed there would be a claim nothing made, so the sentence says
// what was checked instead.
func messageOf(result pipelineResult, data map[string]any) string {
inner, _ := data["status"].(string)
switch {
case result.Status == delegate.StatusPass && inner == "pass":
if commands, checked := data["verification_commands"]; checked && wholeNumber(commands) == 0 {
return "submitted a change; " + nothingToCheck
}
return "submitted a change, and the project's own build and tests passed"
case result.Status == delegate.StatusPass && inner == "pass-unverified":
return "submitted a change, and nothing finished checking it"
Expand Down Expand Up @@ -410,7 +424,7 @@ func observedOf(data map[string]any) string {
case commands > 0:
said = append(said, fmt.Sprintf("the project's %d build and test commands all passed", commands))
default:
said = append(said, "the project has no build or tests it could find to run")
said = append(said, nothingToCheck)
}
if data["suite_dead"] == true {
said = append(said, "its test suite could not even start")
Expand Down
9 changes: 8 additions & 1 deletion internal/seniordev/app/solo_ship.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,16 @@ func (runner *pipeline) soloShip(
// while recording no command at all, so counting commands would call a
// project whose suite was never found -- the vacuous-green shape -- a
// verified pass.
// A project with nothing discoverable to run passes having run no
// command, and its reason says that rather than that anything passed
// (run.go's messageOf says the same of the ending's sentence).
outcome.Status = "pass"
checked := "its build and tests passed"
if len(verification.Commands) == 0 {
checked = "found no build or tests to run"
}
endingReason = fmt.Sprintf(
"submitted, and its build and tests passed: %s (%s)", candidate.describe(), candidate.Reason,
"submitted, and %s: %s (%s)", checked, candidate.describe(), candidate.Reason,
)
default:
// The candidate does not verify. It is still what ships: it is the only
Expand Down
Loading