Start commands in a directory that exists, and decline an ask whose toolchain could not be read (issue 734)

A build module runs in the directory its build was unpacked in. When the
node-engine replaced that build, every command the module started inherited a
deleted working directory, and the toolchain probe's failure became a final
refusal of the check that asked it (three checks on ace on 2026-10-26).

forgekey: ExecRunnerClean starts its command in the HOME it is given, or the
temporary directory; the other runners keep the process's directory while it
exists and use the temporary directory once it is gone.

buildstep: a guard whose probe failed answers a Refusal with Again set; the
puller declines such an ask (delivery.StepDeclined) so its asker puts it again
for another node or a later pull, instead of ending it refused. A version
mismatch stays a refusal.
This commit is contained in:
2026-10-26 15:01:57 +01:00
parent 21252194d5
commit 36061b5c56
7 changed files with 100 additions and 18 deletions
+4 -2
View File
@@ -27,11 +27,13 @@ type Guard struct {
// Check probes the installed version, writes it to <node>.<language> of the toolchains state every time it runs,
// and answers nil when it is the recipe's. Otherwise it answers a *Refusal naming both versions; the installed
// version is written even then, so the indexer sees that builders disagree. A probe that fails is a *Refusal
// naming its failure, and nothing is written. A write that fails is an error that is not a Refusal.
// naming its failure with Again set, and nothing is written: a toolchain that could not be read is not a toolchain
// known to be wrong, so the ask goes to another node or a later pull instead of ending refused (novox/hq issue 734).
// A write that fails is an error that is not a Refusal.
func (g Guard) Check(ctx context.Context) error {
installed, err := g.Probe(ctx)
if err != nil {
return &Refusal{Text: fmt.Sprintf("the installed %s version could not be read: %v", g.Language, err), Want: g.Want}
return &Refusal{Text: fmt.Sprintf("the installed %s version could not be read: %v", g.Language, err), Want: g.Want, Again: true}
}
key, err := delivery.ToolchainKey(g.Node, g.Language)
if err != nil {
+5 -4
View File
@@ -46,7 +46,7 @@ func TestCheckRefusesAnotherVersionAndWritesTheInstalledOne(t *testing.T) {
if !errors.As(err, &ref) {
t.Fatalf("Check = %v, want a Refusal", err)
}
if ref.Text != "go 1.27.1 installed, the recipe builds with go 1.27.0" || ref.Want != "1.27.0" || ref.Have != "1.27.1" {
if ref.Text != "go 1.27.1 installed, the recipe builds with go 1.27.0" || ref.Want != "1.27.0" || ref.Have != "1.27.1" || ref.Again {
t.Errorf("refusal = %+v", ref)
}
if k["ace.go"].Version != "1.27.1" {
@@ -54,13 +54,14 @@ func TestCheckRefusesAnotherVersionAndWritesTheInstalledOne(t *testing.T) {
}
}
// 1473/AC3: a probe that fails: a Refusal naming the failure, and nothing put.
// 1473/AC3: a probe that fails: a Refusal naming the failure, and nothing put. Issue 734: one to ask again (Again), not a
// toolchain known to be wrong.
func TestCheckRefusesAFailedProbeAndPutsNothing(t *testing.T) {
k := toolchainKeys{}
err := guardFor(k, "", errors.New("go: not found")).Check(context.Background())
var ref *Refusal
if !errors.As(err, &ref) || !strings.Contains(ref.Text, "go: not found") {
t.Fatalf("Check = %v, want a Refusal naming the probe's failure", err)
if !errors.As(err, &ref) || !strings.Contains(ref.Text, "go: not found") || !ref.Again {
t.Fatalf("Check = %+v, want a Refusal to ask again naming the probe's failure", err)
}
if len(k) != 0 {
t.Errorf("put %v", k)
+26 -9
View File
@@ -320,22 +320,33 @@ func (p *Puller) next(ctx context.Context, wait time.Duration) (*Pulled, error)
// back, as one whose start cannot be.
func (p *Puller) decline(msg *Pulled, ask delivery.StepAsk, lacks []string) {
step, key, err := begin(p.Put, p.Node, ask.Kind+" ask", ask.ID, AskWhat(ask))
if err != nil {
p.logf("the ask %s cannot be declined on %s, so it is given back: %v", ask.ID, p.Node, err)
_ = msg.Nak()
return
}
why := fmt.Sprintf("the ask %s needs %s, and %s lacks %s: it is left to a node that has it", ask.ID,
strings.Join(ask.Needs, ", "), p.Node, strings.Join(lacks, ", "))
p.endDeclined(msg, ask, key, step, why, lacks)
}
// endDeclined ends a begun step of an ask refused as declined (delivery.StepDeclined) and drops its message, for its
// asker to put again; the worker it came from is not looked at again until the next pull's wait. A message whose
// decline cannot be written is handed back.
func (p *Puller) endDeclined(msg *Pulled, ask delivery.StepAsk, key string, step delivery.Step, why string, lacks []string) {
step.Ended = now().UTC()
step.State, step.Reason = delivery.StepRefused, why
var err error
step.Result, err = json.Marshal(delivery.StepDeclined{Declined: true, Needs: ask.Needs, Lacks: lacks})
if err == nil {
why := Refuse("the ask %s needs %s, and %s lacks %s: it is left to a node that has it", ask.ID,
strings.Join(ask.Needs, ", "), p.Node, strings.Join(lacks, ", "))
step.Ended = now().UTC()
step.State, step.Reason = delivery.StepRefused, why.Error()
step.Result, err = json.Marshal(delivery.StepDeclined{Declined: true, Needs: ask.Needs, Lacks: lacks})
if err == nil {
err = writeEnd(p.Put, key, step)
}
err = writeEnd(p.Put, key, step)
}
if err != nil {
p.logf("the ask %s cannot be declined on %s, so it is given back: %v", ask.ID, p.Node, err)
_ = msg.Nak()
return
}
p.logf("the ask %s is declined on %s: it needs %s, and this node lacks %s", ask.ID, p.Node, strings.Join(ask.Needs, ", "), strings.Join(lacks, ", "))
p.logf("the ask %s is declined on %s: %s", ask.ID, p.Node, why)
_ = msg.Term()
p.mu.Lock()
if p.declined == nil {
@@ -431,6 +442,12 @@ func (p *Puller) run(ctx context.Context, msg *Pulled, ask delivery.StepAsk, key
// The process stops: the ask is neither ended nor acknowledged here, so the stream gives it to a node that runs.
return
}
if ref := (*Refusal)(nil); errors.As(err, &ref) && ref.Again {
// The node cannot run it at this moment (its toolchain could not be read): declined, not refused, so the
// asker puts it again for another node or a later pull (issue 734).
p.endDeclined(msg, ask, key, step, fmt.Sprintf("%s cannot run the ask %s now: %v", p.Node, ask.ID, err), nil)
return
}
if ferr := finish(p.Put, key, step, result, err, p.Logf); ferr != nil {
// No end was written: the ask is not acknowledged as done, so the stream gives it again and a run that can
// write its end (or the poison rule, issue 1907) ends it where its asker sees it (issue 1930).
+17 -1
View File
@@ -139,7 +139,8 @@ func TestPullHoldsAtMostItsCapacity(t *testing.T) {
// An ask that is not one, or not this seat's, is dropped and runs nowhere. And issue 890/AC1: a check ask needing
// node-build-node is declined on a node that lacks it (its step refused as declined, naming what it lacks, the message
// dropped for the asker to put again), and runs on a node that has it.
// dropped for the asker to put again), and runs on a node that has it. And issue 734: an ask whose run is refused to
// ask again (a toolchain that could not be read) is declined the same way, not refused for good.
func TestPullDropsWhatIsNoAsk(t *testing.T) {
src, st := NewFakeAsks(), newSteps()
src.Publish(delivery.AskDelivery, []byte(`{"v":1}`))
@@ -173,6 +174,21 @@ func TestPullDropsWhatIsNoAsk(t *testing.T) {
Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { return nil, nil }}
go q.Pull(ctx)
waitFor(t, "the placed ask acked where its need is met", func() bool { a, _, _ := has.Fates(); return len(a) == 1 })
probe, probeSteps := NewFakeAsks(), newSteps()
unready := testAsk(delivery.AskCheck)
_ = probe.PublishAsk(unready)
r := &Puller{Source: probe, Language: delivery.AskGo, Node: "ace", Put: probeSteps.put, Capacity: func() int { return 1 },
PullWait: 10 * time.Millisecond, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) {
return nil, &Refusal{Text: "the installed go version could not be read: getwd: no such file or directory", Again: true}
}}
go r.Pull(ctx)
waitFor(t, "the unready ask termed", func() bool { _, tm, _ := probe.Fates(); return len(tm) == 1 })
key, _ = delivery.AskStepKey("ace", unready.ID)
s, _ = probeSteps.get(key)
if _, ok := delivery.DeclinedOf(s); !ok || !strings.Contains(s.Reason, "getwd") {
t.Fatalf("the step of an ask its node could not run now = %+v, want it declined naming why", s)
}
}
// load reads the recorded steps state's raw values: the pause key is not a step, so it is kept apart.
+5
View File
@@ -10,6 +10,11 @@ type Refusal struct {
Text string
// Want is the version the recipe asks for and Have the one found; either may be empty.
Want, Have string
// Again says the node cannot run the step at this moment, not that it never should: its toolchain could not even
// be read (novox/hq issue 734: a probe started in a directory deleted under it). A puller declines such an ask
// rather than refusing it (delivery.StepDeclined), so its asker puts it again for another node or a later pull; a
// step run as a tool ends refused, as any Refusal.
Again bool
}
func (r *Refusal) Error() string { return r.Text }
+26 -1
View File
@@ -43,7 +43,8 @@ func ExecRunnerFor(bound time.Duration) Runner {
// ExecRunnerClean is ExecRunnerFor whose commands get exactly the env argument as their environment, and nothing of
// this process's (issue 1497; novox/hq ADR 0466 §4: no registry password in a fetch's environment). The caller
// gives PATH and HOME itself. A directory is the command's own business (`git -C`, `go -C`).
// gives PATH and HOME itself. A directory is the command's own business (`git -C`, `go -C`): it is started in the HOME
// it is given, or the machine's temporary directory, never in this process's (stableDir).
func ExecRunnerClean(bound time.Duration) Runner {
return func(ctx context.Context, env []string, name string, args ...string) Ran {
return execRun(ctx, bound, true, env, name, args...)
@@ -62,6 +63,7 @@ func execRun(ctx context.Context, bound time.Duration, clean bool, env []string,
} else {
cmd.Env = append(os.Environ(), env...)
}
cmd.Dir = stableDir(clean, env)
var out, errb bytes.Buffer
cmd.Stdout, cmd.Stderr = &out, &errb
err := cmd.Run()
@@ -83,6 +85,29 @@ func execRun(ctx context.Context, bound time.Duration, clean bool, env []string,
return r
}
// stableDir is the directory a command starts in when it names none (novox/hq issue 734). **Never one that may be
// deleted under it**: a module runs in the directory its build was unpacked in, which a new build replaces, and a
// command started there inherits it — `go version` then fails "getwd: no such file or directory" before it reads
// anything. A clean command starts in the HOME it is given, or the temporary directory; any other keeps this
// process's directory while it exists, and starts in the temporary directory once it is gone. "" is this process's.
func stableDir(clean bool, env []string) string {
if !clean {
if _, err := os.Getwd(); err == nil {
return ""
}
return os.TempDir()
}
for i := len(env) - 1; i >= 0; i-- {
if home, ok := strings.CutPrefix(env[i], "HOME="); ok {
if info, err := os.Stat(home); err == nil && info.IsDir() {
return home
}
break
}
}
return os.TempDir()
}
func firstLine(s string) string {
for _, l := range strings.Split(s, "\n") {
if l = strings.TrimSpace(l); l != "" {
+17 -1
View File
@@ -48,13 +48,29 @@ func TestExecRunnerForZeroIsCutByItsContext(t *testing.T) {
}
}
// 1497/AC4: ExecRunnerClean(0) runs env with only the given words: PATH and HOME, not the process's secret.
// 1497/AC4: ExecRunnerClean(0) runs env with only the given words: PATH and HOME, not the process's secret. And issue
// 734: run from a process whose own directory was deleted under it (a module whose build a new one replaced), a clean
// command starts in its HOME, and any other in the temporary directory, rather than failing to find where it is.
func TestExecRunnerCleanGivesTheCommandOnlyTheEnvArgument(t *testing.T) {
t.Setenv("MESH_TEST_SECRET", "x")
r := ExecRunnerClean(0)(context.Background(), []string{"PATH=" + os.Getenv("PATH"), "HOME=/h"}, "env")
if r.Status != 0 || !strings.Contains(r.Stdout, "HOME=/h") || !strings.Contains(r.Stdout, "PATH=") || strings.Contains(r.Stdout, "MESH_TEST_SECRET") {
t.Errorf("ran = %+v", r)
}
home, gone := t.TempDir(), t.TempDir()
t.Chdir(gone)
if err := os.Remove(gone); err != nil {
t.Fatal(err)
}
r = ExecRunnerClean(0)(context.Background(), []string{"PATH=" + os.Getenv("PATH"), "HOME=" + home}, "pwd")
if r.Status != 0 || strings.TrimSpace(r.Stdout) != home {
t.Errorf("a clean command run from a deleted directory = %+v, want it started in its HOME %s", r, home)
}
r = ExecRunnerFor(0)(context.Background(), nil, "pwd")
if r.Status != 0 || strings.TrimSpace(r.Stdout) != os.TempDir() {
t.Errorf("a command run from a deleted directory = %+v, want it started in %s", r, os.TempDir())
}
}
// 1497/AC5: ExecRunnerFor(0) is unchanged: the env argument is added to the process's environment.