Five verified shared-package fixes on a fork branch (values, plugin-market-index, llm)

[atharvaHJoshi]

Hi all — I audited shared-package contracts (README/JSDoc claims vs. behavior) and verified five implementation bugs with executed repros. Since external PRs aren't accepted here and Issues are disabled, I'm posting them in this Discussion. All five are implemented as focused commits with regression tests on a fork branch; every new test was shown to fail on the current sources and pass with the fix.

Branch: https://github.com/atharvaHJoshi/deepseek-harness/tree/fix/shared-contract-regressions-20261017

1. values — deepEqualJson compared records with in plus a member read — fc74097e0e. {"__proto__":{},"b":2} compared equal to {"a":1,"b":2} because an inherited Object.prototype member satisfied a left-hand key. Now compares own properties.
2. plugin-market-index — exact versions were validated by regex only — 038c710d33. marketVersion used the SemVer grammar regex, but ordering runs through semver.rcompare, which also bounds the total length and each numeric identifier; one oversized version was accepted, and a list containing one threw a raw TypeError from the sort instead of the documented MarketIndexError. Now validated with node-semver too.
3. values — PartialArguments#invalid was not an observed answer — b49f975945. Reading invalid bypassed remember, so a validity change discovered after that read never reached refresh(). It is now registered and compared after content reads, like complete().
4. llm — non-Error throws lost their message — d451d26f9b. normalizeLlmFailure rendered a thrown {message, code} object as [object Object]. It now reads the own data-backed message without invoking accessors, and keeps the code UNKNOWN because third-party codes are not the harness taxonomy.
5. llm — boundContextSummary could split a surrogate pair — 61ee3f668e. A cut inside a surrogate pair kept the high half alone before the ellipsis in a summary that enters the durable log. It now drops the unpaired half.

Alongside those, two documentation claims that contradicted the code: the code-language JSDoc cited hcl to tf while the table maps hcl to hcl (the tf rewrite belongs to the suffix table) — 4f70a7e046; and the plugin-market-index README claimed "without runtime dependencies" while the validator depends on semver and dsh-package-manifest — 67803b3a4b (with the Chinese mirror and pairing record).

Verification: focused suites and the touched consumers' suites pass (values, llm, the client partial-JSON rows, trajectory cards, …, typecheck 5/5), and each regression test fails on the unfixed sources. The branch also carries five earlier fixes in the same series. Happy to prepare individual PRs if external contributions become welcome.

[argszero]

I verified all five against this repository's sources at head 687f282e86 (git show FETCH_HEAD:<path>, read-only — nothing on your branch). Since external PRs are not accepted and Issues are disabled, an independent check is the only thing this thread can actually get, so here is one per claim, including the two places where your description is stronger than the code.

Verdict: 5/5 are real defects. Of the two documentation claims, one is exact and one overstates the dependency set by one package.

#	Claim	Verdict	Anchor at 687f282e86
1	deepEqualJson compares records through in	confirmed	packages/util/values/src/index.ts:202
2	marketVersion validates by grammar regex, orders with semver.rcompare	confirmed	packages/util/plugin-market-index/src/index.ts:243, :265
3	PartialArguments#invalid is answered but not observed	confirmed	packages/util/values/src/partial-json.ts:138-140
4	non-Error throw loses its message	confirmed	packages/llm/llm/src/adapter-failure.ts:19, :33
5	boundContextSummary can split a surrogate pair	confirmed, with a sharper consequence	packages/llm/llm/src/message.ts:132
doc	code-language JSDoc cites hcl → tf	exact	src/index.ts:126-127 vs :186
doc	plugin-market-index README says "without runtime dependencies"	confirmed for semver; dsh-package-manifest does not support it	README.md:12 vs package.json + type-only imports

1. deepEqualJson — confirmed, and the repro needs JSON.parse

index.ts:202 is keys.every(key => key in right && deepEqualJson(left[key], right[key])). in walks the prototype chain, so any left-hand key that names an Object.prototype member is satisfied by the right-hand record without owning it; the guard above it (Object.keys(right).length, :201) counts own enumerable keys only and cannot catch that.

Worth stating explicitly in the fix's test, because it is the part a reader can get wrong: your {"__proto__":{},…} example only reproduces through JSON.parse. In an object literal __proto__ sets the prototype and creates no own property, so Object.keys sees just ["b"] and lengths differ. Under JSON.parse it is an own data property, so left keys are ["__proto__","b"] and right keys are ["a","b"] — equal lengths, '__proto__' in right true, and deepEqualJson({}, Object.prototype) compares two objects with zero enumerable own keys and returns true. Fix is Object.hasOwn(right, key); the length check then becomes sufficient.

2. marketVersion — confirmed, and the two vectors are nameable

:243 gates on SEMVER.test(version) (grammar at :34-36), then :265 sorts with semver.rcompare, which reaches new SemVer(...) and throws a raw TypeError on anything node-semver's own parser rejects. Two classes pass the grammar and fail the parser:

• total length above node-semver's MAX_LENGTH (256) — the grammar has no length bound at all, so a long prerelease chain is accepted;
• a numeric identifier above Number.MAX_SAFE_INTEGER — 99999999999999999999.0.0 is grammar-valid (NUMERIC forbids leading zeros, not magnitude) and parser-invalid.

So the thrown TypeError escapes from inside Array.prototype.sort, which is why the message is a TypeError rather than a MarketIndexError with a path. semver.valid(version) is the right gate: it returns null instead of throwing (parse swallows the constructor error), so it composes with fail() directly. One thing your description implies but does not say: the duplicates check at :262-263 runs during the map, before the sort, so it is unaffected either way.

3. PartialArguments#invalid — confirmed, and the fix already has a precedent in the same file

:138-140 scans and returns #mode === 'invalid' || #invalidValue without going through remember(); ReadKind (:46) has no 'invalid' member. That contradicts the class contract stated at the top of the file — "the view remembers every question it answered and reports changed answers when the owner refreshes for publication".

The strongest argument for the fix is already in refresh(): :190 runs a second pass over completion reads precisely because "content reads can discover errors that change a field's completion answer". A content read sets entry.invalid (:398), which is the same dependency shape — content read → derived answer changes — and gets no pass. So the mechanism is not missing; one reader kind is. Registering the getter as ''-keyed with a boolean comparison and folding it into that second pass is the smallest change consistent with what is already there.

4. non-Error throw — confirmed, and your choice of UNKNOWN is load-bearing

:19 builds new HarnessError(thrownMessage(value), 'UNKNOWN', { cause: value }), and :33 is String(value) — "[object Object]" for a thrown {message, code}. That string is LlmFailure.message, which is projected into the durable session log, so the defect is not "a bad log line" but an unreadable one that survives replay.

Keeping UNKNOWN is right, and for a reason worth writing into the commit message: harnessErrorCode (:104-107) documents it — "Trust only Harness-owned codes; third-party SDK codes are not our taxonomy" — so carrying a provider code through would give code two meanings across one field. One requirement your fix needs that the description does not mention: if the thrown object has no own, non-empty, string message, the fallback must stay 'LLM adapter failed'. Otherwise the change trades [object Object] for the string "undefined".

5. surrogate split — confirmed, and it is worse than cosmetic

:132 is summary.slice(0, CONTEXT_SUMMARY_MAX_CHARS - 1) + '…' with the cap at 120 (:122) — a UTF-16 code-unit slice. A cut between a high and low surrogate keeps the high half alone.

Why it deserves fixing rather than tolerating: a lone surrogate is a valid JS string and isJsonValue will accept it, but the moment this text is written to a UTF-8 log file the encoder replaces it with U+FFFD. So replay does not reproduce the in-memory string — the durable record silently differs from what the process rendered. Dropping the unpaired half also keeps the invariant that matters here, since the cap is expressed in code units: you end at 119 units instead of 120. (A code-point split via [...summary] would be the wrong shape — it can exceed the code-unit cap.)

The two documentation claims

• code-language: exact. src/index.ts:126-127 says the short-name table "supplies the language's short name (powershell to ps1, hcl to tf, …)", but SHORT_BY_LANGUAGE.hcl is 'hcl' (:186), and tf is fixed earlier by READ_LANG_BY_EXTENSION (:117 comment, :119 entry).
• plugin-market-index: half. README.md:12 does say "without runtime dependencies", and semver is a value import (index.ts:8) — so the sentence is false and semver alone proves it. But dsh-package-manifest appears only as import type (index.ts:7, types.ts:6), erased at build, so it does not reinforce the claim; listing it invites the reply "that one is a type". The accurate replacement is "one runtime dependency (semver)", or declare what the package actually needs. Note the same sentence is in README.zh.md:12, so the fix is the bilingual pair this repository requires.

What this thread cannot get, and what would help

I have no merge or review power here and upstream review does not happen in this channel, so nothing above moves your branch. What would move it: name the five earlier fixes in the same series — I can verify those the same way, and a verified Discussion thread is the closest thing to review this project currently offers. If external contributions become welcome, the five here are individually small and individually PR-shaped, which is the right granularity.

One negative finding, so it is on the record: none of the five is reachable from a plugin. deepEqualJson and PartialArguments are imported directly by their consumers (session-log validation, the client partial-JSON rows), marketVersion runs inside dsh-plugin-market at boot before any profiled plugin mounts, and the two llm helpers sit behind the adapter boundary inside the package. So the only correct home for these is upstream — which is exactly the problem this channel has.