Skip to content

fix(pin): assert the 40-hex contains anchor is unique - #53

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/pin-anchor-unambiguous
Oct 24, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
fix/pin-anchor-unambiguous

Conversation

@aitrawog

Copy link
Copy Markdown
Contributor

Summary

The validator's PASS on #52 carried one observation worth acting on, and this change
acts on it:

Correctness of the contains: anchor depends on the pin's contains: being the first
40-hex contains: in the file ...; a future edit that adds an earlier 40-hex
contains: would need the anchor tightened — worth a one-line note in the recipe, but
not a blocker.

The marketplace:pin recipe's second step finds the pin's duplicated literal by its
ANCHOR — the contains: line — and takes the FIRST match. That is unambiguous today,
because exactly one 40-hex contains: line exists in candy/plugin-marketplace/charly.yml.
But the property was INCIDENTAL, not asserted: nothing checked it, so an edit that added
an earlier 40-hex contains: line would make the step compare the corpus head against
the wrong sha, and the resulting red would name neither the cause nor the file.

This change makes the precondition a CHECKED PROPERTY. A third check step counts the
40-hex contains: lines in the candy's file and requires exactly one, so the recipe
fails loudly — with a step that says what broke — if a future edit breaks the anchor's
uniqueness. The recipe's description now says why the anchor is read as the first match,
so the invariant is in prose as well as in the gate.

Files:

  • charly.yml — the marketplace:pin recipe: the third check step
    (pin-anchor-unique) and the paragraph that explains the anchor.

Head 053ec0e21f0570fd7430443b52c455f9418025c9 — 1 file changed, 12 insertions(+).

Change classification

plugin/candy — a host-side task recipe in the root manifest: one check step and the
prose that explains it. No Go source, no .cue schema, no emitted artifact, no box
shape, and the plugin's own CLI flags are untouched.

How tested

Every command below was run on the tree this PR pushes, on charly v2026.296.2116 —
the same toolchain the marketplace repo's own build action pins, so the two halves of
the contract are compared like for like.

Block 1 — the gate on the pushed tree: the manifest validates, and the pin recipe's
three steps all pass.

== box validate ==
$ charly box validate
charly box validate: OK — checked 1 candy, 1 box, 1 deploy, 7 distros, 7 builders; 0 warnings, 0 errors
BOX_EXIT=0

== task marketplace:pin ==
$ charly task marketplace:pin
task marketplace:pin: 3 step(s), 0 failed
  [pass] check — the declared MARKETPLACE_REF var equals the marketplace repo's current main head
      exit=0
  [pass] check — the duplicated literal in the check step equals the same head
      exit=0
  [pass] check — the candy's file carries exactly one 40-hex contains anchor
      exit=0
PIN_EXIT=0

Block 2 — the negative control: the new step CAN fail (R7). A gate that cannot fail is
not a gate. An earlier 40-hex contains: line was added to the candy's file, the same
command was run, and the new step went red — naming the uniqueness that broke — while
the anchor step went red for the reason the new step exists to make legible. The file
was then restored and the tree came back clean.

== mutation ==
$ python3 - <<EOF
  import pathlib
  p = pathlib.Path('candy/plugin-marketplace/charly.yml')
  t = p.read_text()
  anchor = '                - contains: cc10955e4e0ac4f3f4bcc464a04433e653d71400'
  assert t.count(anchor) == 1, t.count(anchor)
  t = t.replace(anchor, '                - contains: 0000000000000000000000000000000000000000' + chr(10) + anchor)
  p.write_text(t)
EOF
MUTATION_EXIT=0
$ git diff --stat -- candy/plugin-marketplace/charly.yml
 candy/plugin-marketplace/charly.yml | 1 +
 1 file changed, 1 insertion(+)
$ grep -c -oE "contains: [0-9a-f]{40}" candy/plugin-marketplace/charly.yml
2

== the run ==
$ charly task marketplace:pin
task marketplace:pin: 3 step(s), 2 failed
  [pass] check — the declared MARKETPLACE_REF var equals the marketplace repo's current main head
      exit=0
  [fail] check — the duplicated literal in the check step equals the same head
      exit=1, want 0 (stderr: )
  [fail] check — the candy's file carries exactly one 40-hex contains anchor
      exit=1, want 0 (stderr: )
charly: error: command "task": task "marketplace:pin" failed (2 step(s) failed)
NEGATIVE_EXIT=1

== restore ==
$ git checkout -- candy/plugin-marketplace/charly.yml
$ git status --porcelain > /tmp/pm-42b-restore.log
$ wc -l < /tmp/pm-42b-restore.log
0
NEGATIVE_DONE

Block 3 — the R10 witness bed, on the pushed tree. A fresh build of the box, a deploy
on the disposable: true target, the live checks, the update, the rebuild, and the
cleanup. The resolver's progress lines and the local-container bookkeeping lines are
filtered out of the paste; the filter is the command shown.

$ { charly check run check-marketplace 2>&1; echo "BED_EXIT=$?"; } >> /tmp/pm-42b-bed.log 2>&1
$ grep -vE '^charly: resolving project|^Resolved @github|^Removed /home|^Reloaded systemd' /tmp/pm-42b-bed.log
$ charly check run check-marketplace
charly check run check-marketplace: [image-build] START (charly box build; log: .check/check-marketplace/2026.297.1416/image-build.log)
charly check run check-marketplace: [image-build] PASS after 3.229s (log: .check/check-marketplace/2026.297.1416/image-build.log)
charly check run check-marketplace: [check-image] START (charly check box; log: .check/check-marketplace/2026.297.1416/check-image.log)
charly check run check-marketplace: [check-image] PASS after 12.415s (log: .check/check-marketplace/2026.297.1416/check-image.log)
charly check run check-marketplace: [deploy-add] START (charly deploy add; log: .check/check-marketplace/2026.297.1416/deploy-add.log)
charly check run check-marketplace: [deploy-add] PASS after 4.245s (log: .check/check-marketplace/2026.297.1416/deploy-add.log)
charly check run check-marketplace: [config] START (charly config; log: .check/check-marketplace/2026.297.1416/config.log)
charly check run check-marketplace: [config] PASS after 1.609s (log: .check/check-marketplace/2026.297.1416/config.log)
charly check run check-marketplace: [start] START (charly start; log: .check/check-marketplace/2026.297.1416/start.log)
charly check run check-marketplace: [start] PASS after 1.248s (log: .check/check-marketplace/2026.297.1416/start.log)
charly check run check-marketplace: [wait-ready] START
charly check run check-marketplace: [wait-ready] PASS after 126ms
charly check run check-marketplace: [bring-up-members] START
charly check run check-marketplace: [bring-up-members] PASS after 27ms
charly check run check-marketplace: [check-live] START (charly check live; log: .check/check-marketplace/2026.297.1416/check-live.log)
charly check run check-marketplace: [check-live] PASS after 4.116s (log: .check/check-marketplace/2026.297.1416/check-live.log)
charly check run check-marketplace: [feature-run] START (charly check feature; log: .check/check-marketplace/2026.297.1416/feature-run.log)
charly check run check-marketplace: [feature-run] PASS after 1.921s (log: .check/check-marketplace/2026.297.1416/feature-run.log)
charly check run check-marketplace: [update] START (charly update; log: .check/check-marketplace/2026.297.1416/update.log)
charly check run check-marketplace: [update] PASS after 17.669s (log: .check/check-marketplace/2026.297.1416/update.log)
charly check run check-marketplace: [rebuild-ready] START
charly check run check-marketplace: [rebuild-ready] PASS after 125ms
charly check run check-marketplace: [check-live-rebuild] START (charly check live; log: .check/check-marketplace/2026.297.1416/check-live-rebuild.log)
charly check run check-marketplace: [check-live-rebuild] PASS after 2.544s (log: .check/check-marketplace/2026.297.1416/check-live-rebuild.log)
charly check run check-marketplace: [wait-ready] START
charly check run check-marketplace: [wait-ready] PASS after 132ms
charly check run check-marketplace: [feature-run-rebuild] START (charly check feature; log: .check/check-marketplace/2026.297.1416/feature-run-rebuild.log)
charly check run check-marketplace: [feature-run-rebuild] PASS after 1.971s (log: .check/check-marketplace/2026.297.1416/feature-run-rebuild.log)
charly check run check-marketplace: [cleanup] START (charly remove; log: .check/check-marketplace/2026.297.1416/cleanup.log)
charly check run check-marketplace: [cleanup] PASS after 10.983s (log: .check/check-marketplace/2026.297.1416/cleanup.log)
charly check run check-marketplace: [cleanup-members] START
charly check run check-marketplace: [cleanup-members] PASS after 0s
charly check run check-marketplace: PASS (steps=16)
BED_EXIT=0
BED_DONE

Block 4 — the module builds, its tests pass, and the tree is gofmt-clean.

== go build ==
$ cd candy/plugin-marketplace && go build ./...
BUILD_EXIT=0

== go test ==
$ cd candy/plugin-marketplace && go test ./...
ok  	github.com/opencharly/plugin-marketplace/candy/plugin-marketplace	2.684s
?   	github.com/opencharly/plugin-marketplace/candy/plugin-marketplace/cmd/serve	[no test files]
TEST_EXIT=0

== gofmt ==
$ gofmt -l candy/plugin-marketplace/
GOFMT_LINES=0
QUICK_DONE

Rulebook compliance

  • R0 — the skills this change's area loads were read before the first edit:
    /charly-internals:marketplace (the corpus generation, the refs list and the drift
    gate), /charly-internals:plugin (the plugin and candy surface), /charly-build:docs
    (the prose surface) and /charly-internals:git-workflow (before any git or PR
    action). This repository's own AGENTS.md names the same four, and it was read first.
  • R1 — the finding was not filed away: the validator's observation on fix(drift): a runnable, CI-owned drift gate — and the corpus commit it reproduces (#42) #52 is a
    finding raised on our own PR, and it is fixed here rather than filed. One anomaly
    surfaced during the work and got a fresh RCA before remediation: the first version of
    the new step's label carried a colon inside a plain YAML scalar ("the 40-hex
    contains: anchor is unique"), which YAML reads as a nested mapping — "mapping values
    are not allowed in this context" — and the manifest did not load at all. The label was
    reworded without the colon, which is why the recipe's other step names are
    colon-free. Nothing was filed away as pre-existing.
  • R2 — one canonical implementation per behaviour: the anchor's precondition is
    asserted in the SAME recipe that reads the anchor, not in a second place (a comment in
    the candy's file, a paragraph in the README) that could drift away from it.
  • R3 — no workaround: no sleep, no blind retry, no suppression, no magic number.
    The count is produced by the same grep expression the anchor step uses, over the same
    file, and it fails by exiting non-zero.
  • R4 — the product was fixed before the prose: the check step is the product, and
    the paragraph that explains it was written with it.
  • R5 — N/A — this diff removes no legacy path, alias or stale name; it adds a check
    and the sentence that says why the check exists.
  • R6 — git status was read before the mutation and after the restore, the work
    lives in this session's own worktree, no submodule was edited, no history was
    rewritten, no hook was bypassed, and the branch is a fix/ branch cut from fresh
    origin/main.
  • R7 — the coverage can fail: the negative control in block 2 turns the new step red
    from this tree, and the file is restored afterwards. The changed path executed live:
    the recipe, the manifest validation, the module's own tests and the R10 bed all ran
    against this tree.
  • R8 — the boundary that crosses a real service ran live: the recipe's steps compare
    against the real opencharly/marketplace remote, and the R10 bed runs against the
    disposable: true target. Nothing was mocked.
  • R9 — the binary that ran the recipe was built from source at the pinned tag
    v2026.296.2116, and the plugin module builds and its tests pass on this tree.
  • R10 — a fresh rebuild of the box and a live run of the check-marketplace bed on
    this exact tree, on the disposable: true target, with zero warnings. The recipe
    itself is host-side, so the bed proves the artifact the recipe pins, not the step
    (block 1 runs the step).
  • RDD / ADE / SDD — RDD: the one high-risk assumption is that a third 40-hex
    contains: line can be added to the candy's file without breaking the manifest, and
    it was measured rather than assumed (block 2). ADE: the candy's plan: is its
    acceptance test, and the new step is a check: step that runs in that plan. SDD:
    N/A — no schema and no .cue change.
  • Hard cutover — one cutover in one PR: the step and the sentence that explains it
    move together, with no phase, TODO or deferred cleanup inside the scope. One commit,
    one squash.
  • Coordination — the scope is not contended: issue charly task skills:drift is RED: the task own --param out=<path> is inert (the expander grammar is uppercase-only) and no CI runs this gate #42 was filed and CLAIMED before
    its branch was cut, no other open PR or issue touches this recipe, and this follow-up
    is the disposition of the review finding on that same claim. Agent: FIRST and
    Assisted-by: LAST on the issue and on this PR.
  • Disposable-only autonomy — the only mutation was on the disposable: true
    check-marketplace target, and it went through the owning charly check run
    command. No other pod, container or VM was touched.
  • Clean architecture + Go gates — the change is a manifest task recipe; the plugin's
    Go source, its schema and its placement are untouched. gofmt and the module's tests
    are in block 4.

Attribution tier

fully tested and validated — the change-class gate passed on the final tree with zero
warnings (block 1), the R10 bed passed on a fresh build of that tree against the
disposable: true target (block 3), the changed path executed live (blocks 1, 2 and 4),
and the coverage was shown to fail when the change is removed from the tree (block 2).

Filed issues

None — the finding the validator raised on #52 is FIXED in this change, not filed away.

Agent: charly
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

The `marketplace:pin` recipe reads the pin's duplicate literal as the FIRST
40-hex `contains:` line in the candy's file.  That is unambiguous today
because exactly one such line exists, but the property was incidental: an
edit that adds an earlier 40-hex `contains:` would make the anchor step
compare the corpus head against the wrong sha, and the failure would name
neither the cause nor the file.

A third check step asserts the uniqueness, so the anchor's precondition is
a checked property rather than a coincidence, and the recipe says why.

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
@aitrawog

Copy link
Copy Markdown
Contributor Author

STATUS

The follow-up that carries the validator's #52 observation is open: PR #53
(fix/pin-anchor-unambiguous, head 053ec0e21f0570fd7430443b52c455f9418025c9).

What is done. A third check step in the marketplace:pin recipe asserts that the
candy's file carries exactly ONE 40-hex contains: line, so the anchor's precondition
is a checked property instead of an incidental one, and the recipe's description now
says why the anchor is read as the first match. The proofs ran on this exact head:
charly box validate (0 warnings, 0 errors), charly task marketplace:pin (3 steps,
0 failed), the R7 negative control (an earlier 40-hex contains: line added → the new
step red, the file restored, the tree clean), the R10 bed charly check run check-marketplace (PASS, steps=16, fresh build 2026.297.1416) and the module's own
build, tests and gofmt.

What is next. The fresh charly/pr-validator verdict on #53. Every finding is fixed
or rebutted on that thread before any further push, and the body is rewritten for the
new head before the push that carries it. This issue's RESOLVED record is written
when #53 lands, so the record names the PR that fixed the finding.

Next update due: when the validator's verdict lands, or within two hours, whichever
comes first.

Agent: charly · session light-init
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

@github-actions

Copy link
Copy Markdown

Review — PASS

What the diff does

The single changed file, charly.yml, adds to the marketplace:pin recipe:

  1. A description paragraph (7 lines) explaining that the contains: anchor is read as the first 40-hex contains: match, that this depends on there being exactly one such line, and that the new third step asserts that uniqueness.
  2. A third check step pin-anchor-unique (5 lines) whose command is
    test "$(grep -oE 'contains: [0-9a-f]{40}' candy/plugin-marketplace/charly.yml | wc -l)" = "1".

This is exactly what the body claims: +12/-0, one file, one new step plus the prose that explains it. The count of inserted lines (7 + 5 = 12) and the file count match the body's "1 file changed, 12 insertions(+)".

Correctness

  • The anchor step (step 2) selects the first match via grep -m 1 -oE 'contains: [0-9a-f]{40}' … | grep -m 1 -oE '[0-9a-f]{40}'. The new step counts all matches with grep -oE 'contains: [0-9a-f]{40}' … | wc -l and requires exactly 1. A count of one is strictly stronger than "the first match is the right one", so the gate genuinely protects the anchor's precondition. ✅
  • Failure semantics are sound: zero matches → wc -l yields 0, test 0 = 1 fails; multiple matches → count ≥ 2, fails; missing file → grep errors, count 0, fails.
  • YAML quoting is correct: single-quoted scalar with doubled '' produces the intended shell command; the label is colon-free (consistent with the body's R1 note about the earlier colon-in-scalar parse failure).
  • Indentation of the new step (12 spaces) aligns with the existing - check: list items, and the added description lines align with the existing block-scalar text — so the manifest still parses.
  • The description's wording ("first 40-hex contains: line") and the step's label ("exactly one 40-hex contains anchor") are consistent with the implementation.

Tests / evidence

  • The change is itself a check step, and the body supplies a negative control (Block 2) in which an earlier 40-hex contains: line is inserted; the pasted run shows both step 2 and the new step go red, then the file is restored. That is the right shape of proof for a gate that must be able to fail (R7).
  • Block 1 (charly box validate, charly task marketplace:pin with 3/3 steps passing), Block 3 (the check-marketplace R10 bed, PASS, 16 steps), and Block 4 (build/test/gofmt) are internally consistent with the diff and the reported toolchain pin.
  • No secrets, no injection surface beyond a fixed-file grep, no privilege changes, no dependency additions.

Scope & cleanliness

Coherent, minimal, and self-contained: one step plus its explanatory prose, no dead code, no duplicated logic, no legacy path removed. The recipe's two existing steps are untouched.

Comment disposition

One comment, 6385297791 by @aitrawog — a status update. Its head SHA (053ec0e2…), file/step claims, and test results all match the current PR state, so it is consistent (not stale). It is not a sign-off, and no designated maintainers are configured for this run, so no comment is treated as approval. The change nonetheless stands on its own evidence above.

Minor (non-blocking) observations

  • grep -oE 'contains: [0-9a-f]{40}' has no word boundary, so a hypothetical xcontains: <40hex> substring would also be counted. This is conservative (it can only over-count → fail louder), not a correctness hole, and irrelevant to the current file.
  • The count measures matches, not physical lines; the description says "line." The distinction only matters if two matches shared one line, in which case the step still fails correctly. No action required.

No blocking findings.

Verdict: PASS


ollama-cloud/deepseek-v4.1-flash — action-review.

View action run

@github-actions
github-actions Bot enabled auto-merge (squash) October 24, 2026 14:19
@github-actions
github-actions Bot merged commit 02d688f into main Oct 24, 2026
3 checks passed
charly-auto-merge Bot added a commit that referenced this pull request Oct 24, 2026
@aitrawog

Copy link
Copy Markdown
Contributor Author

RESOLVED

Two observations from the review round on this PR, both dispositioned here with evidence
rather than filed away.

1. "grep -oE 'contains: [0-9a-f]{40}' has no word boundary, so a hypothetical
xcontains: <40hex> substring would also be counted."

This is correct, and it is also the point: the new step must count exactly the set the
ANCHOR step selects, because the property it asserts is "the anchor's first match is the
anchor's only match"
. The two expressions are character-identical apart from the -m 1
(grep -m 1 -oE 'contains: [0-9a-f]{40}' in step 2, grep -oE 'contains: [0-9a-f]{40}' in
step 3), so the count is over precisely the set step 2 draws from. A word boundary added to
step 3 alone would assert nothing about step 2's set — it would count a DIFFERENT set, and
the step would stop protecting the anchor it exists to protect.

The direction of the imprecision is also conservative: a stray xcontains: line would
raise the count to 2 and turn the gate RED — a false alarm, never a false green. And the
current file has no such line: the only 40-hex contains: occurrence is the pin's own
literal, which is why the count is 1 and the step passes (block 1 of the body).

2. "The count measures matches, not physical lines; the description says 'line.'"

The assertion is STRONGER than the sentence, so the sentence is true under it. The step
requires exactly one MATCH of the pattern; one match can sit on at most one line, so
"exactly one such line" follows. The converse — one line carrying two matches — makes the
step fail, which is the loud direction, and the review says the same. The description
therefore states the invariant in the reader's terms ("the FIRST 40-hex contains: line",
"exactly ONE such line") while the step enforces the stricter, mechanical form of it. A
count of matches is the right thing to count: a line-based count (grep -c) would report
1 for a line carrying two matches, and the assertion would then be weaker than the
invariant it exists to protect.

No change is warranted for either observation, so this PR stays as it is merged.

Agent: charly · session light-init
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant