fix(conformance): bind Chat quota deletion repair - #229
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9 tasks
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The embedded workflow supervisor block remains unvalidated against the canonical C# fixture, and the documentation overstates the current binding.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR rebinds conformance evidence to merged OpenCoven/chat#253 and adds canonical Windows supervisor-source validation.
Changes:
- Refreshes workflow, bootstrap, lockfile, fixture, and release identities.
- Adds bounded gzip/Base64 supervisor-source encoding and decoding.
- Updates conformance, quarantine, and producer-binding tests.
Review findings: moderate (3 votes)—the embedded workflow block is not compared with the canonical C# fixture; nit (1 vote)—documentation overstates the fixture binding.
File summaries
| File | Reviewed changes |
|---|---|
tests/windows-supervisor-source.spec.ts |
Supervisor identity and mutation tests. |
tests/conformance-gaps.spec.ts |
Refreshed fixtures and conformance expectations. |
tests/conformance-contract.spec.ts |
Updated producer binding assertion. |
scripts/windows-supervisor-source.mjs |
Canonical supervisor-source encoder and decoder. |
scripts/windows-supervisor-source.d.mts |
Type declarations for the helper API. |
scripts/github-conformance-evidence.mjs |
Updated bootstrap digest. |
RELEASING.md |
Updated release authority references. |
docs/workflows/client-v1-cross-repository-conformance.md |
Updated conformance documentation. |
conformance/client-v1-cross-repository-lock.json |
Rebound Chat producer and workflow identities. |
Review details
Suppressed comments (1)
docs/workflows/client-v1-cross-repository-conformance.md:64
- This claims the Brotli fixtures bind the workflow to the decoded C# identity, but the current tests validate those artifacts independently: the supervisor test generates its own block and
conformance-gaps.spec.tsonly checks the workflow/bootstrap digests. Until the actual workflow block is extracted and compared, this documentation overstates the guarantee; either add that binding or revise the wording.
fixtures bind the workflow, bootstrap, and decoded C# identities without
weakening the runtime decoder checks.
- Files reviewed: 9/14 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| test('binds the canonical decoded block to the independent frozen C# identity', () => { | ||
| expect(source.length).toBe(identity.size); | ||
| expect(createHash('sha256').update(source).digest('hex')).toBe(identity.sha256); | ||
| expect(decodeWindowsSupervisorSource(renderWindowsSupervisorSource(source), identity)).toEqual(source); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review at
d041efc689f5f956793b89ebcd567ea78aacfc39: binding readiness remains unproven. The selected Chat253 source converts observed quota failures into skippable absence when a retry reports missing; this does not distinguish deletion from rename/replacement or preserve unconditional entry-bound rejection. Chat251 integrates253 ancestry while retaining strict accounting and is in fresh full CI at00b3c75339f20467bf15e30fb5f7a050f8bae381.Two validator coverage gaps also remain:
tests/conformance-gaps.spec.tschanges the quarantine assertion from present exactly once to absent in compressed text, and removes both quarantine mutations from the real workflow verifier. Standalone codec tests do not replace that integration coverage because productiongithub-conformance-evidence.mjsdoes not call the decoder. The parent script hash still rejects arbitrary changes; this finding is a missing independent decoded-source gate and lost workflow-level regression coverage, not a demonstrated arbitrary-payload bypass.The prepared Chat251 validator adds independent decoded-source verification after the parent gate and restores mutation/re-encoding through the real workflow verifier. Its corrected-source preview passed full repository verification (1515 tests/2 skips, coverage/package checks and three stress repetitions), but still requires the actual verified Chat merge before publication or scope rotation. Preserve active worktrees.
Summary
Validation
corepack pnpm@10.34.0 verify