novox/mesh-catalog
18
0
Fork 0
Code Issues Pull Requests 4 Actions Packages Projects Releases Wiki Activity

mesh-config: apply a held value only at the merged head; a close the forge cannot confirm drops nothing (issue 1826)

Browse Source
This commit is contained in:
jschoubben
2026-10-22 11:07:19 +02:00
parent 260be1c3e6
commit 48b08c74ab
2 changed files with 106 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-config/cmd/mesh-config/pulls.go
+50 -18
View File
@@ -102,7 +102,7 @@ func declDigest(d Declaration) string {
// PullForge is the forge as this module asks it of a pull request: its state, and whether it merged.
type PullForge interface {
Pull(owner, repo string, number int) (open, merged bool, err error)
Pull(owner, repo string, number int) (open, merged bool, head string, err error)
}
// pulls reads what is kept for pull requests, and the held values.
@@ -424,16 +424,16 @@ func (c *Config) keep(p Proposal) (string, error) {
// PullMerged writes each value held for a pull request as the module's own value on its node, with the approval kept
// beside it; it resolves once the trunk declares the key. It answers a line for each.
func (c *Config) PullMerged(pull string) []string {
return c.settlePull(pull, true)
func (c *Config) PullMerged(pull, head string) []string {
return c.settlePull(pull, true, head)
}
// PullClosed drops the values held for a pull request closed unmerged, and says that nothing was set.
func (c *Config) PullClosed(pull string) []string {
return c.settlePull(pull, false)
return c.settlePull(pull, false, "")
}
func (c *Config) settlePull(pull string, merged bool) []string {
func (c *Config) settlePull(pull string, merged bool, head string) []string {
c.mu.Lock()
defer c.mu.Unlock()
var s Sources
@@ -453,6 +453,24 @@ func (c *Config) settlePull(pull string, merged bool) []string {
said = append(said, c.dropKept(kv, kv.Pull+" closed unmerged")...)
continue
}
// **Only a value held at the merged head, whose key that head declares as it was approved** (ADR 0502 clause 7).
why := ""
pd, known := s.Pulls[pullKey(pull)]
d, declared := pd.Declares[kv.Module][kv.Key]
switch {
case head == "":
why = "the merged head is not known, so the value cannot be held to it"
case !strings.EqualFold(head, kv.Head):
why = fmt.Sprintf("it is held at head %s and the pull request merged at %s", short(kv.Head), short(head))
case !known || !declared || declDigest(d) != kv.Digest:
why = "the merged head does not declare the key as the operator was shown"
}
if why != "" {
said = append(said, fmt.Sprintf("%s of %s on %s was held for %s, which merged: dropped, nothing was set (%s)",
kv.Key, kv.Module, kv.Node, kv.Pull, why))
said = append(said, c.dropKept(kv, kv.Pull+" merged, but "+why)...)
continue
}
vk := kv.level().ValueKey(kv.Key)
v := Value{Value: compact(kv.Value), By: kv.By, Via: kv.Via, Why: kv.Why + " (held for " + kv.Pull +
", merged)", Approved: kv.Approved, At: c.now()}
@@ -501,6 +519,8 @@ type PullEvent struct {
Repo string `json:"repo"`
Number int `json:"number"`
Merged bool `json:"merged,omitempty"`
// HeadSHA is the head the pull request merged at (the forge's own word).
HeadSHA string `json:"head_sha,omitempty"`
}
func (p PullEvent) name() string { return fmt.Sprintf("%s/%s#%d", p.Owner, p.Repo, p.Number) }
@@ -511,15 +531,24 @@ func (c *Config) HeardPull(p PullEvent, merged bool) []string {
if p.Owner == "" || p.Repo == "" || p.Number <= 0 {
return nil
}
head := p.HeadSHA
if !merged && !p.Merged && c.Forge != nil {
if open, m, err := c.Forge.Pull(p.Owner, p.Repo, p.Number); err == nil {
if open {
return nil
}
merged = m
// A close is told from a merge by the forge; when it cannot be asked nothing is dropped, and the five-minute
// look finds it.
open, m, h, err := c.Forge.Pull(p.Owner, p.Repo, p.Number)
if err != nil {
c.logf("[mesh-config] %s closed, and the forge does not say whether it merged (%v); the next look decides", p.name(), err)
return nil
}
if open {
return nil
}
merged = m
if head == "" {
head = h
}
}
return c.settlePull(p.name(), merged || p.Merged)
return c.settlePull(p.name(), merged || p.Merged, head)
}
// SweepPulls looks at the forge for every pull request it keeps something for, so a merge or a close it did not hear
@@ -549,13 +578,13 @@ func (c *Config) SweepPulls() []string {
if err != nil {
continue
}
open, merged, err := c.Forge.Pull(owner, repo, n)
open, merged, head, err := c.Forge.Pull(owner, repo, n)
if err != nil {
said = append(said, fmt.Sprintf("the forge does not say whether %s is open: %v", pull, err))
continue
}
if !open {
said = append(said, c.settlePull(pull, merged)...)
said = append(said, c.settlePull(pull, merged, head)...)
}
}
return said
@@ -573,25 +602,28 @@ type toolForge struct {
}
// Pull asks the forge's gitea_get_pull_request whether a pull request is open, and whether it merged.
func (f toolForge) Pull(owner, repo string, number int) (bool, bool, error) {
func (f toolForge) Pull(owner, repo string, number int) (bool, bool, string, error) {
raw, err := f.ask("gitea.gitea_get_pull_request", map[string]any{"owner": owner, "repo": repo, "number": number})
if err != nil {
return false, false, err
return false, false, "", err
}
answer, output, ok := unwrapAnswer(raw)
if !ok {
return false, false, fmt.Errorf("the forge refused gitea_get_pull_request: %s", output)
return false, false, "", fmt.Errorf("the forge refused gitea_get_pull_request: %s", output)
}
var a struct {
Pull struct {
State string `json:"state"`
Merged bool `json:"merged"`
Head struct {
SHA string `json:"sha"`
} `json:"head"`
} `json:"pull"`
}
if err := json.Unmarshal(answer, &a); err != nil || a.Pull.State == "" {
return false, false, fmt.Errorf("the forge's gitea_get_pull_request is not readable: %s", output)
return false, false, "", fmt.Errorf("the forge's gitea_get_pull_request is not readable: %s", output)
}
return a.Pull.State == "open", a.Pull.Merged, nil
return a.Pull.State == "open", a.Pull.Merged, a.Pull.Head.SHA, nil
}
// unwrapAnswer reads a tool's answer whatever wraps it: the protocol's content list, or {output, ok, answer}.
modules/mesh-config/cmd/mesh-config/pulls_test.go
+56 -6
View File
@@ -36,18 +36,19 @@ func aHoldingMesh(t *testing.T) (*Config, *memStore, *standIn, *standForge) {
type standForge struct {
state map[string][2]bool // pull -> open, merged
head string
err error
}
func (f *standForge) Pull(owner, repo string, n int) (bool, bool, error) {
func (f *standForge) Pull(owner, repo string, n int) (bool, bool, string, error) {
if f.err != nil {
return false, false, f.err
return false, false, "", f.err
}
st, ok := f.state[(PullEvent{Owner: owner, Repo: repo, Number: n}).name()]
if !ok {
return true, false, nil
return true, false, f.head, nil
}
return st[0], st[1], nil
return st[0], st[1], f.head, nil
}
func tell(t *testing.T, c *Config, pull, head string, declares map[string]any) *PullDeclaresAnswer {
@@ -318,7 +319,7 @@ func TestTheMergeAppliesAHeldValueAndItResolvesOnceTheTrunkDeclaresTheKey(t *tes
}()
m.events, m.handActs = nil, nil
said := c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434, Merged: true}, true)
said := c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434, Merged: true, HeadSHA: "abc1234def"}, true)
if len(said) == 0 {
t.Fatal("the merge said nothing")
}
@@ -355,7 +356,7 @@ func TestTheMergeAppliesAHeldValueAndItResolvesOnceTheTrunkDeclaresTheKey(t *tes
}
// A merge heard twice sets nothing twice.
m.events = nil
c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434, Merged: true}, true)
c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434, Merged: true, HeadSHA: "abc1234def"}, true)
if len(m.events) != 0 {
t.Errorf("a second merge raised %+v", m.events)
}
@@ -390,6 +391,7 @@ func TestACloseEventForAMergedPullRequestIsItsMerge(t *testing.T) {
tell(t, c, thePull, "abc1234def", n8nPullDeclares())
holdIt(t, c, m, r)
f.state[thePull] = [2]bool{false, true}
f.head = "abc1234def"
c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434}, false)
if v, set := storedValue(t, m, "module.ace.n8n.media-gid"); !set || string(v.Value) != "2000" {
t.Errorf("the merge of a pull request announced closed set %+v", v)
@@ -410,6 +412,7 @@ func TestTheLookAtTheForgeFindsAMissedMergeOrCloseAndLeavesAnOpenPullRequest(t *
t.Errorf("a silent forge: said %v", said)
}
f.err = nil
f.head = "abc1234def"
f.state[thePull] = [2]bool{false, true}
c.SweepPulls()
if v, set := storedValue(t, m, "module.ace.n8n.media-gid"); !set || string(v.Value) != "2000" || keptCount(m) != 0 {
@@ -443,3 +446,50 @@ func TestThePullArgumentsAreRead(t *testing.T) {
}
_ = time.Second
}
// 1826 review r1: the merge applies a value only if it is held at the merged head; otherwise it is dropped and said.
func TestTheMergeAppliesOnlyAValueHeldAtTheMergedHead(t *testing.T) {
for name, head := range map[string]string{"another head": "9999999999", "no head told": ""} {
c, m, r, _ := aHoldingMesh(t)
tell(t, c, thePull, "abc1234def", n8nPullDeclares())
holdIt(t, c, m, r)
said := c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434, Merged: true, HeadSHA: head}, true)
if _, set := storedValue(t, m, "module.ace.n8n.media-gid"); set || keptCount(m) != 0 ||
!strings.Contains(strings.Join(said, "\n"), "dropped, nothing was set") {
t.Errorf("%s: set=%v held=%d said %v", name, set, keptCount(m), said)
}
if len(m.events) == 0 || m.events[len(m.events)-1].Action != ActionDrop {
t.Errorf("%s: raised %+v", name, m.events)
}
}
}
// 1826 review r1: a later head without the declaration drops the value, so the merge has nothing to apply.
func TestALaterHeadWithoutTheDeclarationLeavesNothingToApplyAtTheMerge(t *testing.T) {
c, m, r, _ := aHoldingMesh(t)
tell(t, c, thePull, "abc1234def", n8nPullDeclares())
holdIt(t, c, m, r)
tell(t, c, thePull, "bbbbbbbbbb", map[string]any{})
c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434, Merged: true, HeadSHA: "bbbbbbbbbb"}, true)
if _, set := storedValue(t, m, "module.ace.n8n.media-gid"); set {
t.Error("a value held for a declaration the merged head lacks was set")
}
}
// 1826 review r1: a close the forge cannot be asked about drops nothing; the five-minute look decides.
func TestACloseThatTheForgeCannotBeAskedAboutDropsNothing(t *testing.T) {
c, m, r, f := aHoldingMesh(t)
tell(t, c, thePull, "abc1234def", n8nPullDeclares())
holdIt(t, c, m, r)
f.err = errors.New("the forge is down")
c.HeardPull(PullEvent{Owner: "novox", Repo: "mesh-catalog", Number: 434}, false)
if keptCount(m) != 1 {
t.Fatal("a close the forge could not confirm dropped the value")
}
f.err = nil
f.state[thePull] = [2]bool{false, false}
c.SweepPulls()
if keptCount(m) != 0 {
t.Error("the look did not drop it")
}
}