Skip to content

feat(nodejs): assert the pnpm pin <-> allowBuilds key-form coupling (#5) - #6

Merged
github-actions[bot] merged 2 commits into
mainfrom
feat/harness-bindings
Oct 7, 2026
Merged

github-actions[bot] merged 2 commits into
mainfrom
feat/harness-bindings

Conversation

@aitrawog

@aitrawog aitrawog commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

opencharly/layer-nodejs#5. The pinned PNPM_VERSION and a consumer's allowBuilds KEY FORM
are a coupled pair, and the coupling was invisible from this repo: the pin lived here, the
key form lived in the consumer (pod-dsh), and nothing asserted they agree. A wrong pair
hard-fails the consumer's dsh-web profile install at image build time.

The rule is now declared beside the pin and asserted by the candy's own check, which is this
repo's functional evidence.

  • PNPM_ALLOWBUILDS_GIT_KEY_FORM, PNPM_ALLOWBUILDS_PROBED_BARE and
    PNPM_ALLOWBUILDS_PROBED_URL join PNPM_VERSION in var:, with the measured matrix in the
    comment.
  • One new check: step asserts the pin, the form and the probes agree. A bump across the band
    now goes RED on this repo instead of hard-failing a consumer's build. It accepts only the
    two versions that were MEASURED: any other pin fails loudly and asks the author to re-measure
    and record — the band boundary is deliberately not extrapolated from two endpoints.
  • The nodejs-skill: body mirrors the rule, because the skill entity is the corpus projection
    source (this repo's own rule: a behaviour change not mirrored in the skill leaves the corpus
    stale).

Declared values, quoted from var: in this head:

PNPM_VERSION: 10.33.4
PNPM_ALLOWBUILDS_GIT_KEY_FORM: bare
PNPM_ALLOWBUILDS_PROBED_BARE: 10.33.4
PNPM_ALLOWBUILDS_PROBED_URL: 10.34.6

Correction to the first revision of this PR (the previous head bd28408). That revision
shipped the check with JavaScript template literals (${version}, ${form}, ${want}) inside
its command:, and its comment claimed "No ${...} anywhere". Both were wrong. A check step's
command goes through charly's variable substitution, which reads ${X} as a charly variable and
SKIPS the step when it cannot resolve one; version/form/want are not declared charly
vars, so the step would have been a silent no-op — disabling the very assertion this change
adds, which is the exact class it exists to prevent. The reviewer caught it. The fix is in the
new head: the messages are built by string concatenation, and the command now contains no $
character at all, so the substitution path has no token to act on and the step cannot be skipped.
The comment states the rule and the grep that proves it.

How tested

This repo ships no image and no bed (its AGENTS.md: "this repo has no per-repo candy gate
and ships only .github/workflows/tag-on-merge.yml", and "the candy's plan: check: steps are
the functional evidence"), so the owning evidence is the entity validating plus the changed check
executing. Both, on this head 9ae1184bc59a9216ba9cd47b093e1db153f06c73. charly here is the
worktree-local build 2026.280.1725 in the sibling session worktree:

$ ../opencharly/charly/bin/charly box validate -C .
charly box validate: OK — checked 1 candy, 7 distros, 7 builders; 0 warnings, 0 errors
exit 0

The check is extracted from the committed entity and executed with exactly the var: values
this head declares, so the output comes from the shipped artifact rather than a transcription:

$ cat /tmp/hb-extract.sh
python3 -c 'import yaml,sys; c=yaml.safe_load(open("charly.yml"))["nodejs"]["candy"]; sys.stdout.write("#!/bin/bash\n"+"\n".join("export %s=\"%s\""%(k,v) for k,v in c["var"].items())+"\n"+[s for s in c["plan"] if isinstance(s,dict) and "check" in s and "allowBuilds" in s["check"]][0]["command"])'
$ bash /tmp/hb-extract.sh > /tmp/hb-lc2.sh
$ grep -c '\$' /tmp/hb-lc2.sh
0
$ bash /tmp/hb-lc2.sh
pnpm 10.33.4: allowBuilds git-key form "bare" matches the recorded measurement
exit 0

The grep -c '$' returning 0 is the point of the correction above: the shipped command
carries no substitution token, so the ${...}-skips-the-step failure mode is eliminated by
construction rather than argued. grep exits 1 on zero matches, which is expected here.

The check is falsifiable: it was executed twice more with one drift each, and each fails with
its own message and exit 1:

$ sed 's/PNPM_VERSION="10.33.4"/PNPM_VERSION="10.35.0"/' /tmp/hb-lc2.sh | bash
pnpm 10.35.0 is UNMEASURED for the allowBuilds git-key form. Bump only with a measurement: run a consumer profile that installs a git-specifier dep with a build script and record which form passes, then move PNPM_ALLOWBUILDS_GIT_KEY_FORM and the PROBED_* pair together (opencharly/layer-nodejs#5).
exit 1

$ sed 's/PNPM_ALLOWBUILDS_GIT_KEY_FORM="bare"/PNPM_ALLOWBUILDS_GIT_KEY_FORM="url"/' /tmp/hb-lc2.sh | bash
pnpm 10.33.4 requires the allowBuilds git-key form "bare", but PNPM_ALLOWBUILDS_GIT_KEY_FORM is "url" (opencharly/layer-nodejs#5)
exit 1

That covers both polarities of the pair (the pin moving and the form moving) and both shapes of
drift (an unmeasured pin, and a measured pin paired with the wrong form).

What did NOT run, and the owning follow-on. Proving the pair end to end needs a CONSUMER
build (the dsh-web profile install, e.g. the check-dsh-pod bed in pod-dsh), which cannot be
driven from here: this repo ships no image for charly check box, and the consumer pin can only
advance to a merged ref (Policy B). The step was therefore executed as the runner executes it
— the whole command: block, verbatim, under bash with the var: values exported as ENV — but
NOT through a container build of a consuming image, and I am not claiming otherwise. The follow-on
is the standard producer sequence: this PR merges and tag-on-merge mints the CalVer tag, then the
marketplace corpus is regenerated from the new tag (layer-nodejs refs bump) and the umbrella's
marketplace gitlink advances. Until then the projected corpus carries the OLDER, still-true
skill body — that is the designed behaviour of the corpus, not a divergence.

Rulebook compliance

  • R0 — skills first: /charly-coder:nodejs (the owning skill) and /charly-image:layer (the
    candy-authoring reference) loaded before any entity or plan-step edit, as this repo's
    AGENTS.md mandates; /charly-internals:git-workflow and /charly-internals:strict-policy
    loaded for the landing.
  • R1 — RCA every anomaly: two findings, both fixed in this PR. (a) The re-derivation surfaced a
    divergence that changed the disposition of this issue, so it is corrected on the thread rather
    than silently: the issue's premise was true when filed (2026-10-07T13:35Z) against pod-dsh#24's
    tree, and pod-dsh#28 (merged 20:25Z, tag v2026.280.2025) then vendored the four git-hosted
    plugins as prebuilt file: tarballs, so the profile's own pnpm-workspace.yaml now records the
    coupling as "moot here now that no git dep remains". The SPECIFIC pod-dsh harm is therefore gone;
    the general rule is not, and the producer side carried none of it. (b) The first revision of this
    PR shipped the check with ${...} template literals, which would have made it a silent no-op,
    while its comment claimed otherwise — the correction in ## Summary records it and the new head
    fixes it. No warnings: charly box validate reports 0 warnings, 0 errors.
  • R2 — finish the cutover: the coupling is fixed where it is owned (the pin), the skill body
    moves in the same commit, and there is no TODO, no shim and no follow-up PR inside this repo.
    The consumer leg that remains is a MERGE-SEQUENCED pin bump, which Policy B forbids doing any
    earlier — it cannot be part of this landing by construction, and it is named above rather than
    parked.
  • R3 — no duplication: ONE check: step owns the assertion; the declared constants live once,
    in var:, and are read by the step rather than restated. The rule is written once in the candy
    and mirrored once in the skill body, which is this repo's required projection pair, not a copy.
  • R4 — no workarounds: no sleep, no retry, no fallback branch. The check is a total function of
    the declared values; every drift has a named failure. The var: values are read through
    process.env[...] (exported as ENV before the step) and the messages are built by concatenation,
    so the shipped command: contains no $ character at all — the documented ${...}
    check-step substitution trap, which silently SKIPS the step and trades a red check for an absent
    one, has nothing to act on. That is proven by grep -c '$' -> 0 above, not asserted.
  • R4a — fix the product first: the assertion lands in the candy before the rule is written
    into the skill, and the rule stated is exactly the one the step enforces.
  • R5 — delete legacy completely: nothing is deprecated here — the change adds the missing
    assertion, and the claim-keyed sweep for the retired wording is not applicable because no
    previous claim is contradicted; the skill section is extended in place rather than duplicated.
  • R6 — git safety: git status --short clean after both commits; commits used
    git commit -F <file> throughout, never --no-verify; no force-push, no history rewrite, and
    the fix commit is append-only (the branch was never amended after the push).
  • R7 — prove behaviour, not compilation: pasted above; the changed artifact executed with its
    own declared values and produced its own failure message on two independent drifts, each with
    exit 1, plus the zero-$ proof that the step cannot be skipped.
  • R7a — live or skip: charly box validate and the check ran against the real entity and the
    real node; no mock, stub or fake of any boundary. The one thing not driven is the consumer
    profile install, and it is named as NOT RUN above rather than simulated.
  • R8 — preserve emitted artifacts: the artifact under test is the check step read back out of
    the committed charly.yml (via a YAML parse), not a copy of its text in the PR.
  • R9 — binary equals source: the charly used for box validate is the worktree-local build
    ./charly/bin/charly (2026.280.1725) from the sibling session worktree, not a shared install.
  • R10 — fresh disposable proof: no disposable target is affected by this diff — it changes one
    candy's charly.yml (a var: block, one check: step, and the skill body), with no image, no
    service, no pin and no gitlink in this repo. The owning evidence for a candy change is its
    check: steps, and the changed one ran live. The fresh-rebuild consumer proof is the
    merge-sequenced follow-on named above.

Change classification

Non-.md config change class: git diff --stat $(git merge-base origin/main HEAD)..HEAD lists
charly.yml only (1 file, 83 insertions, 0 deletions), so documentation reviewed is forbidden.
R10 gate: this repo has no per-repo gate; the owning evidence is the entity validating plus the
changed check: step executed live — both pasted above. Attribution tier: analysed on a live
system
— the changed runtime path ran live with retained output, and the full consumer gate did
not pass (it cannot run from this repo).

Relates to #5.

Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system)

The allowBuilds KEY FORM a consumer must use for a GIT-SPECIFIER dependency is
version-coupled: the bare name passes on pnpm 10.33.4 and fails on 10.34.6, the
fully-qualified URL key does the reverse, so no single form works at both. A wrong
pair hard-fails the consumer's profile install at IMAGE BUILD time. The coupling was
invisible from this repo entirely: the pin lived here, the key form lived in the
consumer, and nothing asserted they agree.

- `PNPM_ALLOWBUILDS_GIT_KEY_FORM` plus the two MEASURED probe versions now sit beside
  `PNPM_VERSION`, with the measured matrix in the comment;
- a `check:` step asserts the pin, the form and the probes agree, so a bump that
  forgets them is RED instead of a hard-failed consumer build. It accepts only the two
  probed versions: any other pin must RE-MEASURE the form and record it. The band
  boundary is deliberately NOT extrapolated from the two endpoints, because only those
  two were measured;
- the skill body mirrors the rule, since it is the corpus projection source.

Consumer status: pod-dsh vendored its git-hosted plugins as prebuilt `file:` tarballs
(pod-dsh#28, merged 2026-10-07), so no git dep remains there today. The rule still binds
any consumer that pins one, which is what this change makes assertable from the pin
owner's side.

Proving the pair end to end needs a CONSUMER build; this repo ships no image and no
bed, so what ran here is `charly box validate` plus the check itself executed against
its shipped values and four drift mutations (see the PR body).

Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system)
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Review — BLOCK

Head SHA: bd2840810db3204fac3626238bfd3e17a4a947bc (branch: feat/harness-bindings)

Change class: config — candy charly.yml (var: additions + one check: step) plus the nodejs-skill body. This repo ships no image/bed, so the candy's check: step is the owning evidence.

Blocks

  1. R4 / R7 / A2 — the new check falls into the very ${...} trap it claims to avoid, so under the PR's own model the step is a silent no-op. In charly.yml, the new step's command: block (added ≈lines 149–178 of the new file) contains ${version}, ${form}, and ${want} (JS template literals, e.g. `pnpm ${version} is UNMEASURED for the allowBuilds git-key form…` and `pnpm ${version} requires the allowBuilds git-key form "${want}", but PNPM_ALLOWBUILDS_GIT_KEY_FORM is "${form}"`). The body (R4) and the in-diff comment both assert "No ${...} anywhere" and state that charly's check-step substitution "would read ${X} as a charly variable and SKIP the step on an unresolved one." version/form/want are not declared charly variables, so under the PR's own stated behavior the step is skipped — silently disabling the exact coupling assertion this PR exists to add, which is precisely the failure mode the comment says it prevents. The pasted evidence cannot detect this: it extracts the command and runs it under bash, which performs no charly substitution, so the substitution path is never exercised. Fix: remove every ${...} from the command: (build the messages with string concatenation instead of template literals), and paste output from the step executed through charly's substitution — or supply executed evidence that charly does not skip on these placeholders — then reconcile the "No ${...} anywhere" claim with the shipped command.

Verdict: BLOCK


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

View action run

…where

The reviewer is right, and this is the exact class the step exists to prevent. The new
check's `command:` used JavaScript template literals (`${version}`, `${form}`, `${want}`).
A check step's command goes through charly's variable substitution, which reads `${X}` as
a charly variable and SKIPS the whole step when it cannot resolve one -- and `version`,
`form` and `want` are not declared charly vars. So the step would have been a SILENT NO-OP
in the real runner, disabling the very assertion this change adds, while the comment above
it claimed "No `${...}` anywhere". Both the defect and the false claim are corrected here.

- the messages are now built by string CONCATENATION, so the command contains no `$`
  character at all: `grep -c '$'` over the shipped command returns 0, and with no token to
  resolve the substitution path cannot skip the step;
- the missing-var guard now names WHICH var is missing instead of a generic message;
- the comment states the rule and the grep that proves it, so the claim is checkable
  rather than asserted.

Measured on the shipped entity: `charly box validate` -> 0 warnings, 0 errors; the step
executes and passes at the pin, and both drift mutations (an unmeasured pin; the form
moved with the pin) still exit 1 with their own message.

Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system)
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Review — PASS

Head SHA: 9ae1184bc59a9216ba9cd47b093e1db153f06c73 (branch: feat/harness-bindings)

Security screen: PASS — single candy-config file; no scope mismatch (the var: block, one check: step, and the mirrored skill body are all accounted for), no secrets/egress/network, no guardrail weakening, no dependency/pin/supply-chain change.

Checklist: All applicable rules PASS or NA. The prior head's sole defect is cured: the new check: command: contains zero $ characters (verified line-by-line in the diff — process.env[...] + string concatenation only), so charly's ${...} substitution has no token to act on and the step cannot be silently skipped. Body stat (1 file, +83/−0) matches the diff exactly; check:/exit_status/command YAML nesting is consistent with the sibling plan steps; the assertion logic (measured map {10.33.4→bare, 10.34.6→url}, unmeasured pin → exit 1, form mismatch → exit 1) is internally consistent with the two pasted drift runs.

Comment intake: 6047243077 (@github-actions[bot], head bd28408) — stale-superseded: its only finding (JS template literals ${version}/${form}/${want} in the old command) does not reproduce against the current head, which carries no ${...} token; its "prove it through charly's substitution" demand is moot by construction (nothing to substitute).

Tool-limited (rule 4): the body's assertion that charly exports var: values as ENV (read via process.env[...]) is a runtime claim I cannot re-run, and this repo ships no image for a through-charly execution; the pasted extract-script evidence is internally consistent with the shipped command, and the worst case is a loud failure, not the silent-no-op class this change eliminates.

Verdict: PASS


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

View action run

@github-actions
github-actions Bot enabled auto-merge (squash) October 7, 2026 21:35
@github-actions
github-actions Bot merged commit b25297b into main Oct 7, 2026
1 check passed
charly-auto-merge Bot added a commit that referenced this pull request Oct 7, 2026
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