Fix Fast mode for API key auth#90
Open
Clarence12138 wants to merge 2 commits into
Open
Conversation
Reviewer's GuideRefactors and extends the Fast mode patch script so it can detect and update both legacy negative auth gates and newer positive Flow diagram for updated Fast mode auth gate patchingflowchart TD
Main[Main CLI entry] --> GetPlatforms[GetPlatforms]
GetPlatforms --> FindTargets[FindTargets]
FindTargets -->|targets| ProcessBundles[Loop over bundles]
ProcessBundles --> ProcessBundleFn[ProcessBundle]
ProcessBundleFn --> ParseBundle[ParseBundle]
ParseBundle --> CollectPatches[CollectPatches]
subgraph AuthGateDetection[Auth gate detection]
CollectPatches --> LegacyNeg[CollectLegacyNegativeGatePatches]
CollectPatches --> PositiveGate[CollectPositiveAuthGatePatches]
CollectPatches --> ConditionalGate[CollectConditionalAuthGatePatches]
end
ProcessBundleFn -->|isCheck| ReportCandidates[Print candidate patches]
ProcessBundleFn -->|!isCheck and patches| ApplyPatches[ApplyPatches]
ApplyPatches --> WriteFile[Write patched bundle]
ProcessBundles --> Summary[PrintSummary]
Summary --> Exit[Exit CLI]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
parseBundlenow throws on parse errors where the previous version silently skipped problematic bundles; if mixed/legacy assets are expected, consider restoring thetry/catcharound parsing or handling these errors per-bundle so a single bad asset doesn’t abort the whole script. - Several gate detections (e.g.,
collectLegacyNegativeGatePatches,collectConditionalAuthGatePatches) still rely onsourceFor(...).includes(...); if the bundle format drifts, these string checks may become brittle, so it may be worth tightening them to purely AST-based predicates where possible.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `parseBundle` now throws on parse errors where the previous version silently skipped problematic bundles; if mixed/legacy assets are expected, consider restoring the `try/catch` around parsing or handling these errors per-bundle so a single bad asset doesn’t abort the whole script.
- Several gate detections (e.g., `collectLegacyNegativeGatePatches`, `collectConditionalAuthGatePatches`) still rely on `sourceFor(...).includes(...)`; if the bundle format drifts, these string checks may become brittle, so it may be worth tightening them to purely AST-based predicates where possible.
## Individual Comments
### Comment 1
<location path="scripts/patch-fast-mode.js" line_range="243-253" />
<code_context>
+}
+
+function printSummary({ isCheck, totalCandidates, totalPatched }) {
+ if (isCheck && totalCandidates > 0) {
+ console.log(` [ok] ${totalCandidates} auth gate(s) would be patched`);
+ } else if (isCheck) {
+ console.log(" [ok] no fast_mode auth gates need patching");
+ } else if (totalPatched > 0) {
console.log(` [ok] ${totalPatched} auth gate(s) removed`);
} else {
console.log(" [ok] fast_mode auth gates already patched or absent");
</code_context>
<issue_to_address>
**suggestion:** Summary messages still talk about gates being “removed”, which no longer matches the extended behavior.
Given that we now both remove legacy negative gates and broaden ChatGPT-only gates to include API-key auth, the messages `auth gate(s) removed` / `auth gate(s) would be patched` are misleading. Consider using language like `auth gate(s) patched` (or similar) and differentiating between `totalCandidates` and `totalPatched` in the output so the summary more accurately reflects what actually occurred.
```suggestion
function printSummary({ isCheck, totalCandidates, totalPatched }) {
if (isCheck && totalCandidates > 0) {
console.log(` [ok] ${totalCandidates} auth gate(s) would be patched`);
} else if (isCheck) {
console.log(" [ok] no fast_mode auth gates need patching");
} else if (totalPatched > 0) {
const candidateSuffix =
totalCandidates && totalCandidates !== totalPatched
? ` (out of ${totalCandidates} candidate auth gate(s))`
: "";
console.log(` [ok] ${totalPatched} auth gate(s) patched${candidateSuffix}`);
} else {
console.log(" [ok] fast_mode auth gates already patched or absent");
}
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
本地测试是 OK 的，刚开始开源贡献，如果有不当的地方哈雷佬随时指正 hhh |
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
1 participant
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.
Summary
authMethod === "chatgpt"gates by allowing API key auth too.Notes
chatgpt || apikeyservice-tier gates.Summary by Sourcery
Update the Fast mode patch script to support newer auth gate patterns and improve its robustness and reporting.
Enhancements: