Encode lessons: test-mode loopback guard, publish verification, Changeset present - #64
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🦋 Changeset detectedLatest commit: abba9ad The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Encode this session's recurring corrections as mechanisms instead of prose.
hito the live General thread from a test runassertAllowedCredentialUrl(the one chokepoint every gateway/backend call already passes through) refuses any non-loopback host whenGROK_BOT_TEST=1orNODE_ENV=test, ignoringGROK_BOT_ALLOW_ANY_GATEWAY. Both test runners setGROK_BOT_TEST=1; the route-unit suite asserts the env propagates; a unit test provessendPromptagainsthttps://box.cursor.shthrows beforefetchis called. Fixture sessions moved to loopback URLs.src/core/url-policy.js,scripts/run-unit-tests.mjs,rstest.route-unit.config.ts,test/*changesets/action, whenever no changesets are pending,npm view <name>@<package.json version> versionmust resolve (8 × 15 s window for registry lag) or the job fails. Not gated on the action'spublishedoutput, which is what made silence look green..github/workflows/release.ymlChangeset presentgateChangesetworkflow mirroring agent-bundle:changeset status --since=origin/main, honours theskip-changesetlabel (created) and the machine-ownedchangeset-release/mainbranch.changedFilePatternsscoped to shipped inputs (src/**,package.json, build/ts config, README, LICENSE) so CI-, test-, and lockfile-only PRs need nothing..github/workflows/changeset.yml,.changeset/config.jsonWhy not stronger: an unrepresentable state (a URL type that cannot hold a live host in tests) would need every caller retyped; the runtime refusal sits on the single existing policy function, so no caller can bypass it.
Verification
npm run check(validate, build, validate:artifact, typecheck, 202 unit + 14 route-unit tests): passsrc/core/url-policy.jsreverted, the new tests fail, so they exercise the guardBehavior-test check (would the test pass if every import returned
undefined? no)test/gateway-send.test.js: real loopback HTTP server; asserts the literalsendPromptresult object for a loopback session, then, withGROK_BOT_ALLOW_ANY_GATEWAY=1switched on, the literal refusal message for the non-routablehttps://gateway.invalid(production policy would let it through; test mode must not) and that the server saw exactly the two loopback requests. No mocks asserted.test/url-policy.test.js: asserts the literal normalized URL for loopback (gateway and backend) and the literalRejected … test mode …messages for live hosts, under bothGROK_BOT_TEST=1andNODE_ENV=test; production-policy cases opt out of test mode explicitly.tests/route-unit/tools.test.ts: invokes thegbot_sendMCP tool withhttps://gateway.invalid+GROK_BOT_ALLOW_ANY_GATEWAY=1and asserts the literal tool error text and an empty fake-gateway call log, then invokes it against the loopback fake and asserts the literalstructuredContent. Replaces an env-var pin.npx changeset status --since=origin/main:grok-bot-cli -> 0.4.5via.changeset/test-mode-loopback-only.mdReview
Independent
change-risk-reviewer(Sol) pass: one Medium —changedFilePatternsomittedCHANGELOG.md, whichpackage.json#filesships. Added in the follow-up commit (a hand-edit to the changelog now needs a changeset or the label, which is also the rule). Re-review of the behavior-test rewrite: one Medium — a guard regression would have contacted the realbox.cursor.sh; the tests now target the non-routablegateway.invalidwith the production escape hatch on, so a regression proves itself without leaving the machine. Nothing else remains. No other concrete merge risks: test mode is fail-closed and leaves production policy unchanged; rstest 0.11 propagates config-loaded env to workers;has-changesets,fetch-depth: 0, and bash onwindows-latestconfirmed.