Suriyakumarvijayanayagam
a1e91f776d
Two problems found by deploying the previous commits to production.
FIRST: the rewrite guard compared raw Markdown, so it refused a publish over
formatting. The authoring UI re-serialises a definition when somebody saves it
— writing `webSearch: false` where the hand-authored file omitted the key, and
ordering the frontmatter its own way — and the parser reads absent and false
identically (agent.go: `data["webSearch"] == true`). A definition nobody
meaningfully changed stopped a deploy. Refusing a change that is not a change
is still a bug, even though it fails safe.
definition.SameAgent and SameSkill compare the parsed definition instead, and
are deliberately conservative, because the two ways of being wrong are not
equally bad. A false difference blocks a deploy: visible, recoverable. A false
SAMENESS lets a changed agent overwrite an approved version silently, which is
the thing versioning exists to prevent. So:
- The body is compared verbatim. Agent.Body carries `json:"-"`, so a
comparison that only marshalled the struct would call a completely
rewritten system prompt "unchanged". There is a test that fails loudly on
exactly that, because it is the mistake this design invites.
- List ORDER stays significant. loader.go resolves Skills in order and that
order reaches prompt assembly, so two definitions listing the same skills
differently are still different. A deploy that only reorders still has to
raise its version. That is a limit, recorded in a test rather than left to
be discovered: loosening it needs somebody to decide skill order cannot
matter, which is not a decision to bury in a comparison function.
What it absorbs is exactly what the round trip produces: frontmatter key order,
whitespace, and a defaulted value written out in full.
SECOND: importagents reported "9 agent version(s) recorded" on a run that
recorded nothing. The counter incremented on every successful Snapshot call,
and Snapshot returns nil for the idempotent no-op as well as for a real insert.
The skill counter was already honest; the agent one was not. snapshotAgent now
distinguishes recorded / conflict / already-present, and only the first counts.
Verified locally: 1 on the run that added activity-agent v2, 0 on the re-run,
where it previously said 9. A number that says nine every time is one nobody
checks on the day it matters.
Full suite green against PostgreSQL, only TestLive* skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJvibeSc1JYXjatankqM1g
96 lines
3.2 KiB
Go
96 lines
3.2 KiB
Go
package definition
|
|
|
|
import (
|
|
"bytes"
|
|
"encoding/json"
|
|
)
|
|
|
|
// SameAgent reports whether two agent definitions mean the same thing.
|
|
//
|
|
// This exists because a published version is compared against a new publish to
|
|
// decide whether the new one is a rewrite. Comparing the raw Markdown makes
|
|
// that decision on formatting: the authoring UI re-serialises a definition when
|
|
// somebody saves it — writing `webSearch: false` where the hand-authored file
|
|
// left the key out, and ordering the frontmatter its own way — so a definition
|
|
// that nobody meaningfully changed stops a deploy.
|
|
//
|
|
// The comparison is deliberately conservative, because the two ways of being
|
|
// wrong are not equally bad. Reporting a difference that does not exist blocks
|
|
// a deploy, which is visible and recoverable. Reporting no difference when one
|
|
// exists lets a changed agent overwrite an approved version silently, which is
|
|
// the thing versioning is for. So anything not PROVABLY inert counts as a
|
|
// difference:
|
|
//
|
|
// - The body is compared verbatim. It is the system prompt, and Agent.Body
|
|
// carries `json:"-"`, so marshalling alone would ignore a complete rewrite
|
|
// of the instructions.
|
|
// - List ORDER is significant. loader.go resolves Skills in order, so the
|
|
// order reaches prompt assembly. Two definitions listing the same skills
|
|
// differently are treated as different, and a deploy that only reorders
|
|
// one still has to raise its version. That is a deliberate limit, not an
|
|
// oversight — loosening it needs someone to decide that skill order cannot
|
|
// matter, and that is not a decision to make inside a comparison function.
|
|
//
|
|
// What it does absorb is exactly what the round trip produces: frontmatter key
|
|
// order, whitespace, and a defaulted value written out explicitly.
|
|
func SameAgent(stored, incoming string) bool {
|
|
if stored == incoming {
|
|
return true
|
|
}
|
|
a, err := ParseAgent(stored, Options{})
|
|
if err != nil || a == nil {
|
|
return false
|
|
}
|
|
b, err := ParseAgent(incoming, Options{})
|
|
if err != nil || b == nil {
|
|
return false
|
|
}
|
|
// Body first: it is the expensive thing to get wrong and the cheap thing
|
|
// to check.
|
|
if a.Body != b.Body {
|
|
return false
|
|
}
|
|
ja, err := json.Marshal(a)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
jb, err := json.Marshal(b)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
return bytes.Equal(ja, jb)
|
|
}
|
|
|
|
// SameSkill reports whether two skill definitions mean the same thing.
|
|
//
|
|
// The same reasoning as SameAgent, and the same conservatism. It matters less
|
|
// here — a skill that compares unequal produces a spurious version rather than
|
|
// a blocked deploy, because skills are numbered by the server and have nothing
|
|
// to refuse — but a history full of versions that record a reformat is a
|
|
// history nobody reads.
|
|
func SameSkill(stored, incoming string) bool {
|
|
if stored == incoming {
|
|
return true
|
|
}
|
|
a, err := ParseSkill(stored, Options{})
|
|
if err != nil || a == nil {
|
|
return false
|
|
}
|
|
b, err := ParseSkill(incoming, Options{})
|
|
if err != nil || b == nil {
|
|
return false
|
|
}
|
|
if a.Body != b.Body {
|
|
return false
|
|
}
|
|
ja, err := json.Marshal(a)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
jb, err := json.Marshal(b)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
return bytes.Equal(ja, jb)
|
|
}
|