Test tool#415
Open
c121914yu wants to merge 3 commits intolabring:mainfrom
Open
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9893fcac50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| describe('fetchUrl isInternalAddress', () => { | ||
| it('should not mark public IPv4 as internal', () => { | ||
| expect(isInternalAddress('http://115.238.95.70:38000')).toBe(false); |
There was a problem hiding this comment.
In these new tests isInternalAddress is now async, so this assertion compares a Promise object to false rather than the resolved boolean. The same pattern is used in the private-IP test, so the fetchUrl test file will fail under Vitest until the assertions await the result or use .resolves.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,207 @@ | |||
| import dns from 'dns/promises'; | |||
| import ipaddr from 'ipaddr.js'; | |||
There was a problem hiding this comment.
This new runtime import relies on ipaddr.js, but modules/tool/packages/fetchUrl/package.json does not list it in dependencies. In an npm/yarn production install or isolated tool package build where transitive dependencies are not hoisted, the fetchUrl tool will fail to resolve this module at startup/build time; add ipaddr.js to this package's dependencies.
Useful? React with 👍 / 👎.
|
✅ Build Successful - Preview Images for this PR: Changed packages: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the FastGPT plugin repo across three areas: (1) strengthens fetchUrl SSRF/internal-network detection by introducing a DNS/IP-aware checker, (2) refreshes multiple model-provider presets and rerank model schema, and (3) adds/reshapes contributor “skills” and development guidance documentation.
Changes:
- Replaced
fetchUrl’s internal-address detection with a new async checker (DNS + IP literal variants + metadata endpoint blocking) and added Vitest coverage. - Updated model provider preset lists (new models, removals, rerank
maxTokenadditions) and mademaxTokenrequired for rerank items in validation. - Added
.codex/skills/*tool-dev and model-provider-updater assets and removed legacy.claude/skills/toolDev/*.
Reviewed changes
Copilot reviewed 34 out of 36 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/package.json | Bumps SDK version to 0.6.1. |
| modules/tool/packages/fetchUrl/test/index.test.ts | Adds tests for internal address detection (needs async fix). |
| modules/tool/packages/fetchUrl/src/network.ts | Introduces new async internal-network/metadata detection logic (IPv6 localhost parsing issue + new dependency). |
| modules/tool/packages/fetchUrl/src/index.ts | Switches internal check to new async helper (currently hard-codes internal-IP checking on). |
| modules/model/provider/StepFun/index.ts | Adds/removes StepFun model presets. |
| modules/model/provider/Siliconflow/index.ts | Adds rerank maxToken. |
| modules/model/provider/Qwen/index.ts | Adds rerank presets and maxToken fields. |
| modules/model/provider/OpenAI/index.ts | Adds/removes OpenAI model presets; minor formatting cleanup. |
| modules/model/provider/Moonshot/index.ts | Removes deprecated kimi-latest-* presets. |
| modules/model/provider/MistralAI/index.ts | Adds several new Mistral presets. |
| modules/model/provider/Jina/index.ts | Adds rerank maxToken fields. |
| modules/model/provider/Groq/index.ts | Adds new Groq presets (LLM/STT). |
| modules/model/provider/Grok/index.ts | Adds new Grok model preset. |
| modules/model/provider/Gemini/index.ts | Reorders/updates Gemini model IDs and prunes older experimental entries. |
| modules/model/provider/Claude/index.ts | Adds new Claude model presets. |
| modules/model/provider/BAAI/index.ts | Adds rerank maxToken. |
| modules/model/provider/AliCloud/index.ts | Adds AliCloud STT preset. |
| lib/validates/model.ts | Makes rerank models require maxToken in schema. |
| AGENTS.md | Adds contributor/dev workflow documentation for this repo. |
| .codex/skills/tool-dev/SKILL.md | Adds tool-dev skill docs (command path needs correction). |
| .codex/skills/tool-dev/scripts/validate_tool_package.mjs | Adds a structural validation script for tool packages. |
| .codex/skills/tool-dev/references/requirement_template.md | Adds tool design template (command path needs correction). |
| .codex/skills/tool-dev/references/design_spec.md | Adds tool design reference docs. |
| .codex/skills/tool-dev/agents/openai.yaml | Adds agent metadata for tool-dev skill. |
| .codex/skills/model-provider-updater/SKILL.md | Adds model-provider-updater docs (command paths need correction). |
| .codex/skills/model-provider-updater/scripts/model_provider_presets.mjs | Adds provider inventory/plan/apply script (usage output path needs correction). |
| .codex/skills/model-provider-updater/references/provider_sources.json | Adds official source hints per provider. |
| .codex/skills/model-provider-updater/references/plan.example.json | Adds example plan format. |
| .codex/skills/model-provider-updater/agents/openai.yaml | Adds agent metadata for model-provider-updater skill. |
| .claude/skills/toolDev/SKILL.md | Removes legacy Claude skill docs. |
| .claude/skills/toolDev/scripts/quick_validate.py | Removes legacy validation script. |
| .claude/skills/toolDev/scripts/package_skill.py | Removes legacy packaging script. |
| .claude/skills/toolDev/scripts/init_skill.py | Removes legacy initializer script. |
| .claude/skills/toolDev/references/requirement_template.md | Removes legacy requirement template. |
| .claude/skills/toolDev/references/design_spec.md | Removes legacy design spec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+11
| it('should not mark public IPv4 as internal', () => { | ||
| expect(isInternalAddress('http://115.238.95.70:38000')).toBe(false); | ||
| }); | ||
|
|
||
| it('should mark private IPv4 as internal', () => { | ||
| expect(isInternalAddress('http://192.168.1.20:8080')).toBe(true); | ||
| }); |
Comment on lines
+11
to
+13
| export const isInternalAddress = createInternalAddressChecker({ | ||
| checkInternalIp: () => true | ||
| }).isInternalAddress; |
Comment on lines
+156
to
+161
| const hostDomain = normalizeDomain(parsedUrl.hostname); | ||
| const localHost = serviceLocalHost.split(':')[0].toLowerCase(); | ||
|
|
||
| // 1. localhost / 本机 | ||
| if (LOCALHOST_HOSTNAMES.has(hostDomain) || hostDomain === localHost) { | ||
| return true; |
Comment on lines
+1
to
+3
| import dns from 'dns/promises'; | ||
| import ipaddr from 'ipaddr.js'; | ||
| import { isIPv6 } from 'net'; |
| Run it before final validation for new or heavily edited tool packages: | ||
|
|
||
| ```bash | ||
| node .agents/skills/tool-dev/scripts/validate_tool_package.mjs modules/tool/packages/<toolName> |
|
|
||
| ## Validation | ||
|
|
||
| - `node .agents/skills/tool-dev/scripts/validate_tool_package.mjs modules/tool/packages/<toolName>` |
Comment on lines
+13
to
+48
| 1. Inventory the current registry. | ||
|
|
||
| ```bash | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs inventory | ||
| ``` | ||
|
|
||
| Use `--json` when another script or a temporary comparison file needs structured output. | ||
|
|
||
| 2. Check providers one by one against official sources. | ||
|
|
||
| Start from `references/provider_sources.json`, but verify the current official page/API during the run because model catalogs change often. Use official provider docs, official pricing/model pages, or official model-list APIs. Do not use third-party blogs, search snippets, or aggregator pages as removal evidence unless the provider is that aggregator, such as OpenRouter. | ||
|
|
||
| `check-sources` only checks whether source-hint URLs are reachable enough to use as starting points. A 403 access-limited result can still be acceptable for docs that block automated requests, and a passing source check is not evidence that the provider catalog was audited. | ||
|
|
||
| 3. Classify differences conservatively. | ||
|
|
||
| Add a preset when an official source lists a model that is absent locally and it belongs to an existing provider. Clone the closest existing preset in the same provider and family, then adjust context, output limit, vision, reasoning, tool calling, response-format, and field-map fields from official docs or the closest local pattern. | ||
|
|
||
| Remove a preset only when an official source explicitly marks the model as deprecated, retired, unavailable, or when an authoritative model-list API/document states that only the returned/listed models are supported and the local model is absent. Do not delete local custom placeholders in `Other`, `Ollama`, `HuggingFace`, `OpenRouter`, or similar open catalogs unless the provider explicitly removes that exact model from its own official API/catalog. | ||
|
|
||
| Remove preview, experimental, or dated candidate presets when the same model family has a stable public model ID in the same provider and official docs describe the preview/experimental ID as deprecated, superseded, unavailable, or no longer recommended. Do not remove a preview ID solely because a stable-looking sibling exists; keep it when official docs still list it as current, required for a distinct capability, or the stable ID has not reached the same capability. | ||
|
|
||
| 4. Create and apply an update plan. | ||
|
|
||
| Generate a template and fill only confirmed additions/removals: | ||
|
|
||
| ```bash | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs plan-template --provider OpenAI > /tmp/model-provider-plan.json | ||
| ``` | ||
|
|
||
| Dry-run before writing: | ||
|
|
||
| ```bash | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs apply-plan --plan /tmp/model-provider-plan.json --dry-run | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs apply-plan --plan /tmp/model-provider-plan.json --write | ||
| ``` |
Comment on lines
+845
to
+856
| function printUsage() { | ||
| console.log(`Usage: | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs inventory [--provider OpenAI] [--json] | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs plan-template [--provider OpenAI] | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs apply-plan --plan /tmp/plan.json (--dry-run | --write) [--json] | ||
| node .agents/skills/model-provider-updater/scripts/model_provider_presets.mjs check-sources [--provider OpenAI] [--strict] [--json] [--concurrency 4] | ||
|
|
||
| Options: | ||
| --repo-root <path> Override the FastGPT plugin repository root. | ||
| --provider <name> Limit to one provider or a comma-separated list. | ||
| --concurrency <n> Limit concurrent source URL checks. | ||
| `); |
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.
No description provided.