Conversation
Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com>
The frontend visual picker previously generated CSS selectors using the direct child combinator (`>`). This caused matching failures when the backend `goquery` parser processed raw HTML, as the frontend DOM often contains structural differences due to client-side hydration (e.g., Astro custom elements) or automatic tag insertion. Changed the combinator in `getCssSelector` from `>` to a space (descendant combinator) to make the generated `item_selector` robust and fault-tolerant against these DOM differences. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com>
…#666) * docs: add System Runtime observability tool documentation Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> * docs: add System Runtime observability tool documentation Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
- Introduced `currentLink` ref to hold the generated recipe feed URL - Created a dedicated clipboard instance for copying the link - Updated `handleCopyLink` to set the link and perform the copy action - Simplified the copy configuration logic with separate clipboard instances
- Hide TopicFeed routes and UI until feature is ready to prevent navigation to incomplete pages.
- Remove hardcoded `time.Now()` for Updated and Created in TopicFeed.Fetch - Add `applyTopicFeedTimestamps` helper to set feed timestamps based on article metadata - Apply helper after merging feeds and after processing feeds Ensures topic feed metadata accurately reflects the most recent article timestamps, improving consistency across feeds.
- Add frontend API wrapper for feed preview - Add better error handling in UI
- Allow preview requests to specify a craft name and apply craft transformations - Build Atom XML and parse with gofeed to feed craft processor - Update error handling to differentiate invalid craft names and URL resolution - Extend API client to accept craftName and display errors in UI - Refactor feed comparison component to use previewFeed API and show error alerts
Bumps [pnpm/action-setup](https://github.com/pnpm/action-setup) from 3 to 6. - [Release notes](https://github.com/pnpm/action-setup/releases) - [Commits](pnpm/action-setup@v3...v6) --- updated-dependencies: - dependency-name: pnpm/action-setup dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
## What Changed 💡 - Updated Topic Feed sections in concepts.md for 0ba68e8 ## Why 📖 Recent code changes require documentation sync as the Topic Feed UI route has been temporarily hidden. ## Files Updated | File | Changes | |------|---------| | doc-site/src/content/docs/en/guides/start/concepts.md | Added caution callout | | doc-site/src/content/docs/zh/guides/start/concepts.md | Added caution callout | | doc-site/src/content/docs/zh-tw/guides/start/concepts.md | Added caution callout | ## Verification 🔍 - [x] Build succeeds - [x] Links checked Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com>
Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 6.18.0 to 7.1.0. - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](docker/build-push-action@2634353...bcafcac) --- updated-dependencies: - dependency-name: docker/build-push-action dependency-version: 7.1.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
* feat(admin): highlight missing crafts on health dashboard Extract missing nodes and prominently display them at the top of the Dependency Check page using a styled warning card with red tags, improving UX/UI visibility of missing crafts. Additionally, updates missing craft labels within the main dependency tree to be styled with a bold, red font weight to highlight errors inline. Also moved the analyze button to the upper right card actions slot for better layout hierarchy and added i18n support in EN/ZH-CN. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> * style: auto-format code --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* docs: improve curl-to-rss template documentation Added a new "Using Templates" section to the `curl-to-rss` documentation guides across all three locales (English, Simplified Chinese, Traditional Chinese). This update details the available variables (`.Fields`, `.Item`), built-in template functions (`trimSpace`, `trim`, `default`), and provides concrete examples for common use cases like URL concatenation and fallback values to help users better utilize the feature. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> * style: auto-format code --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…face{}`, it unmarshals numbers into `float64` by default. If this `float64` is a large number (like a post ID) and is later used in string interpolation (e.g., via `text/template`), it gets converted into scientific notation (e.g., `2.658732e+06`).
This commit updates `parser.ParseJSONItems` and the `curl_to_rss` parsing controller to use `json.NewDecoder` and `.UseNumber()`. This forces JSON numbers to be unmarshaled as `json.Number` (a string-backed type). Since `json.Number` implements `fmt.Stringer`, it interpolates safely without altering its string representation.
Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com>
…714) * fix: improve browserless error handling and feed viewer presentation - Add detailed logging of the full response body in `GetBrowserlessContent` for debugging. - Truncate the response string returned from `GetBrowserlessContent` to avoid massive error traces breaking UIs. - Update `classifyFeedViewerError` to correctly parse underlying errors by unwrapping the batch failure prefix. - Add `humanizeBrowserlessStatus` to provide a clean, human-readable error to the user when browserless extraction fails, ensuring the RSS client properly renders the error item instead of receiving a hard 500 status. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> * fix: resolve golangci-lint S1017 unconditionally use strings.TrimPrefix - Simplified the `strings.HasPrefix` and `strings.TrimPrefix` logic in `classifyFeedViewerError` to unconditionally use `strings.TrimPrefix`, resolving the golangci-lint S1017 warning and passing CI checks. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
- Replace MD5 hashing with FNV-1a for article and cache key generation across LLM, craft, and DAO modules - Introduce `GetTextContentHash` for content hashing and `GetPasswordMD5Hash` for admin passwords - Update cache key logic to use the new hash function - Remove unused fmt import from `guid.go` - Adjust related unit tests to match new hashing behavior
- Introduced test_redis_helper_test.go providing setupTestRedis helper for miniredis - Updated benchmark and unit tests to use setupTestRedis instead of direct miniredis usage - Removed redundant fmt and miniredis imports from benchmark_test.go - Simplified cache seeding with redis.SetString helper - Adjusted test data to include unique identifiers for clarity and isolation - Cleaned up test setup and environment variable configuration for consistency
Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com>
Added quotes around FC_HTTP_USER_AGENT_FEED and FC_HTTP_USER_AGENT_HTML in .env.example since they contain spaces and parentheses. Also updated the english and chinese customization.md files to include a note about quoting variables if they have spaces or parentheses. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com>
- Replaced `context.Background()` with a 30-second timeout context in `NewCraftedFeedFromUrl` to prevent indefinite hangs. - Added `retry.Context(ctx)` to the `retry.Do` loop in `HttpFetcher.Fetch` to respect the context timeout. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com>
* feat: add io.LimitReader to prevent OOM in HTTP fetcher Limits the maximum amount of data read into memory during an HTTP fetch to a fixed size of 10MB to mitigate Out-Of-Memory (OOM) risks from unbounded or maliciously large response bodies. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> * fix: return explicit error on limit exceed in HTTP fetcher Limits the maximum amount of data read into memory during an HTTP fetch to a fixed size of 10MB to mitigate Out-Of-Memory (OOM) risks from unbounded or maliciously large response bodies. Now explicitly returns an error if this limit is exceeded. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* fix: properly return 404 when deleting non-existent recipes and flows - Update `internal/dao/recipe.go` (`DeleteCustomRecipeV2`) to explicitly check for `RowsAffected == 0` and return `gorm.ErrRecordNotFound`. - Update `internal/controller/custom_recipe.go` (`DeleteCustomRecipe`) to catch `gorm.ErrRecordNotFound` and return an HTTP 404 response. - Update `internal/controller/craft_flow.go` (`DeleteCraftFlow`) to check `RowsAffected == 0` after GORM `.Delete()` execution and return an HTTP 404 response. - These changes resolve a frontend bug where deleting a non-existent recipe or flow returned a silent 200 OK from the backend, leading to unpredictable UI states on subsequent renders. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> * fix: properly return 404 when deleting non-existent recipes and flows - Update `internal/dao/recipe.go` (`DeleteCustomRecipeV2`) to explicitly check for `RowsAffected == 0` and return `gorm.ErrRecordNotFound`. - Update `internal/controller/custom_recipe.go` (`DeleteCustomRecipe`) to catch `gorm.ErrRecordNotFound` and return an HTTP 404 response. - Update `internal/controller/craft_flow.go` (`DeleteCraftFlow`) to check `RowsAffected == 0` after GORM `.Delete()` execution and return an HTTP 404 response. - These changes resolve a frontend bug where deleting a non-existent recipe or flow returned a silent 200 OK from the backend, leading to unpredictable UI states on subsequent renders. Co-authored-by: Colin-XKL <49122401+Colin-XKL@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideRenames and expands the Curl-to-RSS feature into a JSON-to-RSS tool across UI and docs, introduces a unified feed preview API for the admin RSS viewer and feed compare tools, hardens and tunes HTTP fetching (user-agents, retry, response limits) and LLM/cache utilities, improves Topic feed timestamps and various tests, and adds observability, UX, and documentation refinements plus small API behavior fixes. Sequence diagram for the new unified feed preview flowsequenceDiagram
actor AdminUser
participant FeedViewerPage
participant FeedComparePage
participant AdminAPI
participant FeedViewerController
participant SourceFactory
participant RssSource
participant HttpFetcher
participant ExternalFeed
participant CraftEngine
AdminUser->>FeedViewerPage: Enter feedUrl and click Fetch
FeedViewerPage->>AdminAPI: GET /api/admin/tools/feed/preview?input_url&craft_name=proxy
AdminAPI->>FeedViewerController: PreviewFeedViewer(req)
FeedViewerController->>FeedViewerController: validateFeedViewerURL(inputUrl)
FeedViewerController->>SourceFactory: Get(SourceRSS)
SourceFactory-->>FeedViewerController: rssSourceFactory
FeedViewerController->>RssSource: Fetch(ctx)
RssSource->>HttpFetcher: Fetch(ctx)
HttpFetcher->>HttpFetcher: resolveRequestProfile(config)
loop retryAttempts
HttpFetcher->>ExternalFeed: HTTP request with UserAgent DefaultFeedUserAgent
ExternalFeed-->>HttpFetcher: HTTP response (limited by MaxResponseBodySize)
end
HttpFetcher-->>RssSource: []byte body
RssSource-->>FeedViewerController: CraftFeed feed
alt craft_name is proxy or empty
FeedViewerController-->>AdminAPI: APIResponse FeedViewerPreview
else craft_name is custom
FeedViewerController->>CraftEngine: ProcessFeed(parsedFeed, inputUrl, craftName)
CraftEngine-->>FeedViewerController: feeds.Feed craftedFeed
FeedViewerController-->>AdminAPI: APIResponse FeedViewerPreview (crafted)
end
AdminAPI-->>FeedViewerPage: FeedViewerPreview JSON
FeedViewerPage->>FeedViewerPage: Render FeedViewContainer
AdminUser->>FeedComparePage: Enter feedUrl, select craft, click Compare
FeedComparePage->>AdminAPI: GET /api/admin/tools/feed/preview?input_url&craft_name=proxy
FeedComparePage->>AdminAPI: GET /api/admin/tools/feed/preview?input_url&craft_name=selectedCraft
AdminAPI-->>FeedComparePage: Original FeedViewerPreview or error
AdminAPI-->>FeedComparePage: Crafted FeedViewerPreview or error
FeedComparePage->>FeedComparePage: Show side-by-side feeds and error alerts
Class diagram for updated HttpFetcher and request profilingclassDiagram
class HttpFetcherConfig {
+string URL
+string Method
+map~string string~ Headers
+string Body
+bool UseBrowserless
+string Purpose
}
class requestProfile {
+map~string string~ defaultHeaders
+uint retryAttempts
}
class HttpFetcher {
+HttpFetcherConfig* Config
+Fetch(ctx context.Context) []byte
+doRequest(ctx context.Context, profile requestProfile) []byte
+BaseURL() string
}
class fetchError {
-error err
-bool retryable
+Error() string
+Unwrap() error
}
class util_Env {
+DefaultFeedUserAgent() string
+DefaultHTMLUserAgent() string
}
class config_Constants {
+string HttpFetcherPurposeFeed
+string HttpFetcherPurposeHTML
}
class retryPackage {
<<external>>
+Do(operation func() error, options ...) error
}
class httpPackage {
<<external>>
+Client
+MethodGet string
+StatusTooManyRequests int
+StatusInternalServerError int
}
class util_HttpHelpers {
+HTMLDefaultHeaders() map~string string~
+resolveRequestProfile(cfg *HttpFetcherConfig) requestProfile
+isRetryableFetchError(err error) bool
+isRetryableStatus(statusCode int) bool
}
HttpFetcher --> HttpFetcherConfig : uses
HttpFetcher --> requestProfile : uses
HttpFetcher --> fetchError : returns
HttpFetcher --> util_HttpHelpers : calls
HttpFetcher --> httpPackage : uses
HttpFetcher --> retryPackage : uses
util_HttpHelpers --> util_Env : uses
util_HttpHelpers --> config_Constants : uses
class util_Hash {
+GetTextContentHash(text string) string
+GetPasswordMD5Hash(text string) string
}
class CraftTranslateCacheKeyGenerators {
+cacheKeyForArticleTitle(item *feeds.Item) string
+cacheKeyForArticleContent(item *feeds.Item) string
+cacheKeyForArticleLink(item *feeds.Item) string
}
class CraftContentCacheKeyGenerators {
+cacheKeyForCraftArticleContent(article *CraftArticle) string
+cacheKeyForCraftArticleLink(article *CraftArticle) string
}
class CraftLLMCacheKeyGenerators {
+newArticleTitleContentCacheKeyGenerator(prompt string) ArticleCacheKeyGenerator
}
class AdapterLLM {
+CallLLMUsingContext(prompt string, context string, option ContentProcessOption) string
}
class DaoMigrate {
+createAdminUser(db *gorm.DB)
+ResetAdminPassword() error
}
CraftTranslateCacheKeyGenerators --> util_Hash : uses
CraftContentCacheKeyGenerators --> util_Hash : uses
CraftLLMCacheKeyGenerators --> util_Hash : uses
AdapterLLM --> util_Hash : uses
DaoMigrate --> util_Hash : uses
Class diagram for new feed viewer preview controller and frontend typesclassDiagram
class FeedViewerPreviewReq {
+string InputURL
+string CraftName
}
class FeedViewerPreviewImage {
+string URL
+string Title
}
class FeedViewerPreviewItem {
+string GUID
+string Title
+string Link
+string PubDate
+string IsoDate
+string Content
+string ContentSnippet
}
class FeedViewerPreview {
+string Title
+string Description
+string Link
+string FeedURL
+string Copyright
+FeedViewerPreviewImage* Image
+[]FeedViewerPreviewItem Items
}
class PreviewFeedOptions {
+string CraftName
}
class FeedViewerApi {
+previewFeed(inputUrl string, options PreviewFeedOptions) Promise
}
class FeedViewerController {
+PreviewFeedViewer(c *gin.Context)
-loadFeedViewerPreview(c *gin.Context, req FeedViewerPreviewReq) *CraftFeed
-buildCraftPreview(feed *CraftFeed, inputURL string, craftName string) *CraftFeed
-buildFeedViewerPreview(feed *CraftFeed, inputURL string) FeedViewerPreview
-validateFeedViewerURL(rawURL string) error
-classifyFeedViewerError(err error) (int, string)
-humanizeBrowserlessStatus(msg string) string
-humanizeFeedViewerHTTPStatus(msg string) string
}
class SourceConfig {
+string Type
+HttpFetcherConfig* HttpFetcher
}
class SourceFactory {
+Get(sourceType string) func(cfg *SourceConfig) (Source, error)
}
class Source {
<<interface>>
+Fetch(ctx context.Context) (*CraftFeed, error)
}
class CraftFeed {
+string Title
+string Description
+string Link
+string Copyright
+string ImageURL
+string ImageTitle
+[]*CraftArticle Articles
+ToFeedsFeed() *feeds.Feed
}
class CraftArticle {
+string Id
+string Title
+string Link
+string Content
+string Description
+time.Time Created
+time.Time Updated
}
class CraftEngine {
+ProcessFeed(parsedFeed *gofeed.Feed, inputURL string, craftName string) (*feeds.Feed, error)
}
class RouterRegistry {
+RegisterRouters(router *gin.Engine)
}
class AdminUI_FeedViewerPage {
+feedUrl string
+feedContent FeedViewerPreview
+errorMessage string
+fetchFeed()
}
class AdminUI_FeedComparePage {
+feedUrl string
+selectedCraft string
+originalFeedContent FeedViewerPreview
+craftAppliedFeedContent FeedViewerPreview
+originalFeedError string
+craftAppliedFeedError string
+compareFeeds()
+clearErrors()
}
FeedViewerController --> FeedViewerPreviewReq : uses
FeedViewerController --> FeedViewerPreview : returns
FeedViewerPreview --> FeedViewerPreviewImage : contains
FeedViewerPreview --> FeedViewerPreviewItem : contains
FeedViewerController --> SourceConfig : builds
FeedViewerController --> SourceFactory : uses
FeedViewerController --> Source : uses
FeedViewerController --> CraftFeed : uses
FeedViewerController --> CraftEngine : uses
RouterRegistry --> FeedViewerController : registers
AdminUI_FeedViewerPage --> FeedViewerApi : calls
AdminUI_FeedComparePage --> FeedViewerApi : calls
FeedViewerApi --> FeedViewerPreview : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 2 medium |
🟢 Metrics 124 complexity · 14 duplication
Metric Results Complexity 124 Duplication 14
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| func GetPasswordMD5Hash(text string) string { | ||
| h := md5.New() | ||
| h.Write([]byte(text)) | ||
| _, _ = h.Write([]byte(text)) |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
html_to_rss/utils/selector.ts, changing the selector join from' > 'to' 'significantly alters the CSS semantics (direct child vs. any descendant) and may break existing stored selectors; consider either keeping the original combinator or making this behavior explicitly configurable/migrated. - The new feed viewer error handling (e.g.,
classifyFeedViewerError/humanize*helpers) relies heavily on substring matching against error messages, which is brittle as upstream error text changes; consider introducing typed/sentinel errors or error wrapping with well‑defined kinds to drive user-facing messages instead of parsing strings. - The new retrying HTTP fetcher still uses
http.DefaultClientwithout an explicit timeout, which combined with retries can lead to long-hanging requests; consider wiring a dedicatedhttp.Clientwith sane timeouts (and possibly per-purpose settings) to make retry behavior more predictable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `html_to_rss/utils/selector.ts`, changing the selector join from `' > '` to `' '` significantly alters the CSS semantics (direct child vs. any descendant) and may break existing stored selectors; consider either keeping the original combinator or making this behavior explicitly configurable/migrated.
- The new feed viewer error handling (e.g., `classifyFeedViewerError` / `humanize*` helpers) relies heavily on substring matching against error messages, which is brittle as upstream error text changes; consider introducing typed/sentinel errors or error wrapping with well‑defined kinds to drive user-facing messages instead of parsing strings.
- The new retrying HTTP fetcher still uses `http.DefaultClient` without an explicit timeout, which combined with retries can lead to long-hanging requests; consider wiring a dedicated `http.Client` with sane timeouts (and possibly per-purpose settings) to make retry behavior more predictable.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new Feed Viewer tool for previewing RSS feeds, renames "CURL to RSS" to "JSON to RSS" across the documentation and UI, and enhances the HTTP fetcher with retry logic and response size limits. It also improves system health reporting, adds documentation for the System Runtime dashboard, and temporarily hides the Topic Feed feature. Feedback focuses on several technical risks: the switch to FNV-64a for article GUIDs increases collision probability, and MD5 is still being used for password hashing despite being cryptographically broken. Furthermore, the feed preview implementation includes an inefficient serialization round-trip, and the retry logic for HTTP 429 errors is overly aggressive and should respect rate-limiting headers.
Comment on lines
+9
to
+13
| func GetTextContentHash(text string) string { | ||
| h := fnv.New64a() | ||
| _, _ = h.Write([]byte(text)) | ||
| return hex.EncodeToString(h.Sum(nil)) | ||
| } |
Contributor
There was a problem hiding this comment.
Switching from MD5 (128-bit) to FNV-64a (64-bit) for article GUIDs and cache keys significantly increases the risk of hash collisions. In an RSS aggregator, a GUID collision causes different articles to be treated as the same, leading to missing content in news readers. While FNV is faster, the performance gain is negligible compared to the reliability risk. Consider using a stronger hash like SHA-256 or reverting to MD5 for these identifiers.
Comment on lines
+15
to
19
| func GetPasswordMD5Hash(text string) string { | ||
| h := md5.New() | ||
| h.Write([]byte(text)) | ||
| _, _ = h.Write([]byte(text)) | ||
| return hex.EncodeToString(h.Sum(nil)) | ||
| } |
Contributor
There was a problem hiding this comment.
MD5 is cryptographically broken and should not be used for password hashing. It is vulnerable to collision attacks and can be cracked extremely quickly using modern hardware. It is highly recommended to use a modern, slow hashing algorithm like bcrypt, scrypt, or Argon2 to securely store user credentials.
Comment on lines
+112
to
+129
| func buildCraftPreview(feed *model.CraftFeed, inputURL, craftName string) (*model.CraftFeed, error) { | ||
| atomXML, err := feed.ToFeedsFeed().ToAtom() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| parsedFeed, err := gofeed.NewParser().ParseString(atomXML) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| craftedFeed, err := craft.ProcessFeed(parsedFeed, inputURL, craftName) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return model.FromFeedsFeed(craftedFeed), nil | ||
| } |
Contributor
There was a problem hiding this comment.
The buildCraftPreview function performs an inefficient round-trip by serializing the feed to Atom XML and then parsing it back into a gofeed.Feed object. This involves unnecessary CPU and memory overhead. It would be better to refactor the logic to avoid this serialization step, perhaps by providing a direct conversion between the internal model and the gofeed representation.
Comment on lines
+173
to
+175
| func isRetryableStatus(statusCode int) bool { | ||
| return statusCode == http.StatusTooManyRequests || statusCode >= http.StatusInternalServerError | ||
| } |
Contributor
There was a problem hiding this comment.
Retrying on HTTP 429 (Too Many Requests) without respecting the Retry-After header or using exponential backoff is aggressive and potentially counter-productive. A fixed 300ms delay is likely too short for most rate limits and may lead to stricter penalties from target servers. Retries for 429 should ideally be handled by checking the Retry-After header or using a more conservative backoff strategy.
WalkthroughThis PR introduces configurable HTTP User-Agent defaults for feed vs. HTML requests, renames the "CURL to RSS" feature to "JSON to RSS" with Go template processing support, adds a feed preview endpoint for validation, documents System Runtime observability features and Topic Feed development status, migrates content hashing from MD5 to FNV-1a, and enhances error handling in deletion operations. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as Browser
participant Controller as PreviewFeedViewer Controller
participant HttpFetcher as HTTP Fetcher
participant Parser as RSS/Atom Parser
participant Craft as Craft Processor
participant Response as API Response
User->>Client: Enter URL + optional craft_name
Client->>Controller: GET /api/admin/tools/feed/preview?input_url=...&craft_name=...
Controller->>Controller: Validate URL (HTTP/HTTPS, not loopback)
Controller->>HttpFetcher: Fetch feed with Purpose=Feed
HttpFetcher->>HttpFetcher: Build request with User-Agent header
HttpFetcher->>HttpFetcher: Execute with retries
HttpFetcher->>Controller: Raw feed content (RSS/Atom XML)
Controller->>Parser: Parse XML to intermediate feed
Parser->>Controller: Parsed feed object
alt craft_name provided and not "proxy"
Controller->>Craft: ProcessFeed(parsed_feed)
Craft->>Controller: Transformed articles
end
Controller->>Controller: Build FeedViewerPreview response<br/>(metadata + article details)
Controller->>Response: HTTP 200 + FeedViewerPreview JSON
Response->>Client: Feed preview with title, link, items
Client->>User: Display preview
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/craft/guid.go (1)
23-30:⚠️ Potential issue | 🟡 MinorStale comment and one-time GUID churn after upgrade
- Line 23 still says "生成md5作为guid" but the implementation now delegates to
util.GetTextContentHash(FNV-1a per the PR summary). Please update the comment to reflect the new hash function.- Because the hash algorithm changed, the generated GUIDs for items that were previously hashed with MD5 will differ after this upgrade. Downstream subscribers (RSS readers) keyed on GUID may re-surface every existing item as "new" exactly once after the rollout. Worth calling out in release notes/migration guidance, and confirming this is acceptable for GUID stability.
Comment-only fix
-// 根据feed 中文章标题和内容生成md5作为guid, 如果几个字段都为空则返回随机值 +// 根据feed 中文章标题和内容生成内容哈希(GetTextContentHash)作为guid, 如果几个字段都为空则返回随机值 func feedItemGuidGenerator(item *feeds.Item) (string, error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/craft/guid.go` around lines 23 - 30, Update the stale comment on feedItemGuidGenerator to reflect that it now uses util.GetTextContentHash (FNV-1a) instead of MD5, change the Chinese text "生成md5作为guid" to accurately state the hash algorithm used, and add a short note in the comment that switching hashes will change GUIDs (causing a one-time re-surface of items for downstream subscribers) so maintainers see the migration impact; reference the function name feedItemGuidGenerator and util.GetTextContentHash when making this comment-only change and also ensure you document the GUID churn in release notes or migration guidance.web/admin/src/views/dashboard/curl_to_rss/curl_to_rss.vue (1)
814-823:⚠️ Potential issue | 🟡 MinorMisleading i18n key
saveFailedreused for non-save operations.
jsonToRss.msg.saveFailedis shown whenhandleParseCurlfails to parse a curl string (Line 816) and whenhandlePreviewfails to parse JSON (Line 912). Neither is a save operation, so users will see a "Save failed" toast in unrelated contexts. Introduce dedicated keys such asjsonToRss.msg.curlParseFailedandjsonToRss.msg.previewFailedfor clarity.🛠️ Suggested change
- } catch (err) { - Message.error( - t('jsonToRss.msg.saveFailed', { - msg: err instanceof Error ? err.message : String(err), - }) - ); + } catch (err) { + Message.error( + t('jsonToRss.msg.curlParseFailed', { + msg: err instanceof Error ? err.message : String(err), + }) + ); } finally { parsingCurl.value = false; }- } catch (err) { - Message.error( - t('jsonToRss.msg.saveFailed', { - msg: err instanceof Error ? err.message : String(err), - }) - ); + } catch (err) { + Message.error( + t('jsonToRss.msg.previewFailed', { + msg: err instanceof Error ? err.message : String(err), + }) + ); } finally { parsing.value = false; }Also applies to: 910-919
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin/src/views/dashboard/curl_to_rss/curl_to_rss.vue` around lines 814 - 823, The toast uses the generic i18n key jsonToRss.msg.saveFailed in non-save contexts; update the error messages in handleParseCurl and handlePreview to use distinct keys (e.g., jsonToRss.msg.curlParseFailed and jsonToRss.msg.previewFailed) and add those keys to the locale files, then replace Message.error(t('jsonToRss.msg.saveFailed', ...)) calls inside handleParseCurl and handlePreview with the appropriate new key (preserve the existing interpolation of err.message or String(err) and keep parsingCurl.value/preview parsing flags unchanged).
♻️ Duplicate comments (1)
web/admin/src/locale/en-US/jsonToRss.ts (1)
91-91:⚠️ Potential issue | 🟡 MinorInvalid JSON shown as body placeholder.
Same issue as the zh-CN locale:
{ 'foo': 'bar' }isn’t valid JSON. Use double-quoted keys/values so the placeholder illustrates a well-formed body.✏️ Proposed fix
- 'jsonToRss.placeholder.body': "{ 'foo': 'bar' }", + 'jsonToRss.placeholder.body': '{ "foo": "bar" }',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin/src/locale/en-US/jsonToRss.ts` at line 91, The placeholder value for the locale key 'jsonToRss.placeholder.body' contains invalid JSON using single quotes; update the string to show a well-formed JSON example with double quotes (e.g., use {"foo":"bar"} style) so the UI displays a valid JSON body placeholder for users.
🧹 Nitpick comments (22)
proposal/inbox_source_design.md (1)
155-155: Optional: Grammar refinement for clarity.The static analysis tool suggests "删除最旧地记录" (using 地 with the adverb) instead of "删除最旧的记录", following stricter Chinese grammar conventions. However, the current phrasing is commonly used and perfectly understandable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proposal/inbox_source_design.md` at line 155, 句子“滚动清理：查询该 inbox 当前总条数，若超过 `max_items`，删除最旧的记录使总数 = `max_items`”中的“删除最旧的记录”可按静态分析建议改为更严谨的“删除最旧地记录”；请在 proposal/inbox_source_design.md 中将该片段（包含“滚动清理：查询该 inbox 当前总条数，若超过 `max_items`，删除最旧的记录使总数 = `max_items`”）替换为使用“删除最旧地记录”的表述以满足严格语法检查。.github/workflows/docker-publish.yml (1)
102-102: Major bump todocker/build-push-action@v7— all checks pass; optional companion action upgrades available.
- v7.1.0 is stable and the pinned SHA
bcafcacb16a39f128d818304e6c9c0c18556b85fcorrectly resolves to the v7.1.0 tag.- v7.0.0 switched the default runtime to Node 24, which requires Actions Runner v2.327.1 or later. This workflow runs on
ubuntu-latestand doesn't use the deprecatedDOCKER_BUILD_NO_SUMMARYorDOCKER_BUILD_EXPORT_RETENTION_DAYSenvs, so no action is needed here — but if this workflow ever runs on a self-hosted runner, ensure it's on runner v2.327.1 or later.- None of the inputs used in this step (
file,context,push,tags,labels,build-args,platforms,cache-from/to,provenance) changed between v6 and v7, so the step continues to work as expected.- Optional: Docker's v7 documentation pairs this action with
docker/setup-buildx-action@v4,docker/login-action@v4, anddocker/metadata-action@v6. Your workflow is on v3/v5 — not required, but worth aligning when you next review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-publish.yml at line 102, The workflow step currently pins docker/build-push-action to a commit SHA for v7.1.0; update the uses entry (the line containing uses: docker/build-push-action@bcafcacb16a39f128d818304e6c9c0c18556b85f) to a tagged reference like docker/build-push-action@v7 (or `@v7.1.0`) to make the intent clearer and easier to maintain, and if you expect this to run on self-hosted runners add a note or guard to ensure runner version >= v2.327.1; optionally consider aligning companion actions (docker/setup-buildx-action, docker/login-action, docker/metadata-action) to their v4/v6 releases in the same workflow when you next update.web/admin/src/views/dashboard/custom_recipe/custom_recipe.vue (1)
286-286: Optional: dropcurrentLinkand pass the URL directly tocopyLink.
currentLinkis never rendered or consumed outsidehandleCopyLink; it exists only as a transient buffer souseClipboard's reactivesourcecan pick it up.@vueuse/core^9.3.0'scopyaccepts an optionaltextargument that overrides the bound source, so you can simplify by removing the ref and source binding from the second hook.That said, keeping a separate
useClipboardinstance for the link (so the config modal'scopiedindicator at line 236 isn't toggled by link copies) is a reasonable architectural choice — feel free to keep the split and just remove the ref.♻️ Proposed simplification
- const currentLink = ref('');const { copy: copyConfig, copied } = useClipboard({ source: currentConfig, legacy: true, copiedDuring: 1500, }); - const { copy: copyLink } = useClipboard({ - source: currentLink, - legacy: true, - copiedDuring: 1500, - }); + const { copy: copyLink } = useClipboard({ legacy: true });const handleCopyLink = async (id: string) => { try { - currentLink.value = buildRecipeFeedUrl(id); - await copyLink(); + await copyLink(buildRecipeFeedUrl(id)); Message.success(t('customRecipe.copied')); } catch (e: any) { Message.error(t('customRecipe.copyFailed', { msg: e.message || e })); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin/src/views/dashboard/custom_recipe/custom_recipe.vue` at line 286, currentLink is an unused transient ref; remove the ref and either call the useClipboard copy() method with the URL directly from handleCopyLink (i.e. call copyLink.copy(url) or copy(url)) instead of setting currentLink, or if you want to keep a separate clipboard instance for UI isolation (so the config modal's copied flag isn't affected), keep the second useClipboard but drop currentLink and the source binding—invoke its copy(...) with the URL inside handleCopyLink; update references to currentLink accordingly and ensure copied (the modal's clipboard state) remains tied only to the intended clipboard instance..env.example (1)
18-20: Optional: drop unnecessary quotes on the FEED user-agent to satisfy dotenv-linter.
dotenv-linterflagged the quote characters on line 19 (QuoteCharacter). The quotes are necessary on line 20 because the HTML UA value contains spaces and parentheses, but"FeedCraft/2.0"has no special characters and can be unquoted. This will silence the static-analysis warning without affecting behavior.Proposed tweak
-FC_HTTP_USER_AGENT_FEED="FeedCraft/2.0" +FC_HTTP_USER_AGENT_FEED=FeedCraft/2.0 FC_HTTP_USER_AGENT_HTML="Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/135.0.0.0 Safari/537.36"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 18 - 20, Remove the unnecessary double quotes around the FC_HTTP_USER_AGENT_FEED value in the .env example to satisfy dotenv-linter's QuoteCharacter rule; edit the FC_HTTP_USER_AGENT_FEED entry (symbol: FC_HTTP_USER_AGENT_FEED) so it is unquoted (FeedCraft/2.0) while leaving FC_HTTP_USER_AGENT_HTML quoted because it contains spaces and parentheses.internal/util/env_var.go (2)
14-28: Minor: each call re-creates aviper.ViperviaGetEnvClient().
DefaultFeedUserAgent()/DefaultHTMLUserAgent()are likely called on every outbound HTTP request path. Each call constructs a freshviper.Viper, callsSetEnvPrefix, andAutomaticEnv(see lines 30-38). For a hot path this is unnecessary overhead; consider caching a package-levelviperinstance (or readingos.Getenv("FC_HTTP_USER_AGENT_FEED")directly withstrings.TrimSpace, since no precedence/config-file layering is used here).Not a correctness issue — flagging as optional cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/util/env_var.go` around lines 14 - 28, Both DefaultFeedUserAgent and DefaultHTMLUserAgent call GetEnvClient() which recreates a viper.Viper on each invocation; cache the env client or read the env directly to avoid repeated viper construction. Update the code so GetEnvClient() returns a package-level singleton viper instance (initialized once), or change DefaultFeedUserAgent/DefaultHTMLUserAgent to call os.Getenv("FC_HTTP_USER_AGENT_FEED") and os.Getenv("FC_HTTP_USER_AGENT_HTML") with strings.TrimSpace instead of GetEnvClient(), ensuring you preserve the same fallback defaults (defaultFeedUserAgent and htmlDefaultUserAgent) used now.
9-12: Nit: inconsistent naming between the two defaults.
defaultFeedUserAgentputs “default” as the prefix, whilehtmlDefaultUserAgentputs it as an infix. Align them for readability.Proposed rename
const ( defaultFeedUserAgent = "FeedCraft/2.0" - htmlDefaultUserAgent = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/135.0.0.0 Safari/537.36" + defaultHTMLUserAgent = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/135.0.0.0 Safari/537.36" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/util/env_var.go` around lines 9 - 12, The two constant names are inconsistent: defaultFeedUserAgent vs htmlDefaultUserAgent; rename htmlDefaultUserAgent to defaultHTMLUserAgent (or defaultHtmlUserAgent following your project's initialism convention) so both start with the "default" prefix, and update all references to htmlDefaultUserAgent throughout the codebase (including tests and any docs) to the new constant name to avoid breaking imports/usages; ensure the constant declaration for htmlDefaultUserAgent is replaced with the new name in the same const block.internal/config/source_config.go (1)
7-22: Optional: makePurposea typed alias for stronger type safety.Since
HttpFetcherPurposeFeed/HttpFetcherPurposeHTMLrepresent an enum-like closed set, consider defining a dedicated string type so that callers can’t accidentally pass arbitrary strings and the compiler can catch typos at call sites.Proposed refactor
-const ( - HttpFetcherPurposeFeed = "feed" - HttpFetcherPurposeHTML = "html" -) +type HttpFetcherPurpose string + +const ( + HttpFetcherPurposeFeed HttpFetcherPurpose = "feed" + HttpFetcherPurposeHTML HttpFetcherPurpose = "html" +) @@ - Purpose string `json:"purpose,omitempty"` + Purpose HttpFetcherPurpose `json:"purpose,omitempty"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/source_config.go` around lines 7 - 22, Introduce a dedicated string alias type for the fetcher purpose (e.g., type HttpFetcherPurpose string) and change the constants HttpFetcherPurposeFeed and HttpFetcherPurposeHTML to be of that type; then update the HttpFetcherConfig.Purpose field from string to HttpFetcherPurpose so the compiler enforces allowed values and reduces accidental typos (keep the same constant names but typed, and update any call sites that set Purpose to use the typed constants).internal/util/hash.go (1)
15-19: The password scheme is actually salted; consider bcrypt or argon2id for future migrations.The code uses
GetPasswordMD5Hashonly during admin user initialization ininternal/dao/migrate.go. The actual password storage is more secure than the comment suggested: passwords are stored as SHA256(MD5(password) + salt) with per-user 256-bit random salts fromcrypto/rand. This is not the weak "unsalted MD5" described in the original comment.However, while the salt mitigates rainbow tables and reduces brute-force risk, SHA256 with salt remains suboptimal by modern standards. Consider migrating to
golang.org/x/crypto/bcryptorargon2idin a future refactor, which use password-specific key derivation:import "golang.org/x/crypto/bcrypt" func HashPassword(plain string) (string, error) { return bcrypt.GenerateFromPassword([]byte(plain), bcrypt.DefaultCost) } func VerifyPassword(hash, plain string) bool { return bcrypt.CompareHashAndPassword([]byte(hash), []byte(plain)) == nil }Since password hashes are compared directly throughout the codebase (migrate.go, user.go), any full migration would need versioned hash types and rehash-on-login. This is a reasonable future improvement but not an immediate security issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/util/hash.go` around lines 15 - 19, GetPasswordMD5Hash is currently producing an MD5 digest used by the admin initialization path (see internal/dao/migrate.go) but the codebase actually stores passwords as SHA256(MD5(password)+salt); replace this scheme in a future migration by adding a password hashing layer using a modern KDF (bcrypt or argon2id) via new functions like HashPassword(plain string) (string, error) and VerifyPassword(hash, plain string) bool, update migrate.go and user.go to accept/emit versioned hash formats and implement rehash-on-login logic (detect old MD5+SHA256 hashes, verify them, then rehash with the new KDF and store the versioned hash). Ensure any new API points reference GetPasswordMD5Hash only for backward-compatibility and remove its use once all accounts are migrated.doc-site/src/content/docs/zh-tw/guides/advanced/html-to-rss.md (1)
14-14: Prefer a relative link for same-locale document references.Both
html-to-rss.mdandjson-to-rss/live underdoc-site/src/content/docs/zh-tw/guides/advanced/, so this should use a relative link rather than the absolute/zh-tw/...path.📝 Proposed fix
-此工具專為 HTML 頁面設計。如果你需要處理 JSON API,請使用 [從 JSON 生成 RSS](/zh-tw/guides/advanced/json-to-rss/)。 +此工具專為 HTML 頁面設計。如果你需要處理 JSON API,請使用 [從 JSON 生成 RSS](./json-to-rss/)。As per coding guidelines: "Prefer relative links (e.g., ../advanced/customization) when linking between documents within the same language tree" and "Use absolute links starting with the locale (e.g., /zh-tw/guides/start/quick-start/) only when necessary, such as in the index page or cross-locale references".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc-site/src/content/docs/zh-tw/guides/advanced/html-to-rss.md` at line 14, Change the absolute locale-prefixed link in html-to-rss.md to a relative one: replace the "/zh-tw/guides/advanced/json-to-rss/" link used in the sentence "如果你需要處理 JSON API，請使用 [從 JSON 生成 RSS](/zh-tw/guides/advanced/json-to-rss/)" with a relative path that points to the sibling document (e.g., "../advanced/json-to-rss/" or simply "json-to-rss/" depending on current folder structure) so the internal same-locale reference follows the guideline; update the link target in the file doc-site/src/content/docs/zh-tw/guides/advanced/html-to-rss.md accordingly.internal/craft/llm_processors.go (1)
328-336: Consider inlining the raw prompt rather than re-hashingpromptHash.
promptHashis already a hash, then it's concatenated with title/content and hashed again. IfGetTextContentHashis a narrow hash (e.g., 32-bit FNV-1a), this compounds collision risk — two different prompts that collide onpromptHashproduce identical cache keys for the same article, so e.g. a summary prompt's cached output could be served for a translate prompt. Prefer concatenating the rawpromptdirectly so entropy comes from the original text:♻️ Suggested refactor
func newArticleTitleContentCacheKeyGenerator(prompt string) ArticleCacheKeyGenerator { - promptHash := util.GetTextContentHash(prompt) return func(article *model.CraftArticle) (string, error) { payloadHash := util.GetTextContentHash(strings.Join([]string{ - promptHash, + prompt, strings.TrimSpace(article.Title), strings.TrimSpace(getPrimaryArticleContent(article)), }, "|")) return payloadHash, nil } }Width verification tracked under the
translate.gocomment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/craft/llm_processors.go` around lines 328 - 336, The cache key construction currently uses a pre-hashed prompt (promptHash) which is then re-hashed, increasing collision risk; change the closure returned by the function so it concatenates the raw prompt string (not promptHash) with strings.TrimSpace(article.Title) and strings.TrimSpace(getPrimaryArticleContent(article)) and then calls util.GetTextContentHash on that combined string to produce payloadHash. Update the closure surrounding variables if necessary to capture the original prompt variable and ensure trimming/normalization is applied consistently before hashing so different prompts produce distinct cache keys.internal/dao/recipe.go (1)
86-96: LGTM — correctly distinguishes "not found" from actual DB errors.Callers can now map
gorm.ErrRecordNotFoundto HTTP 404 reliably.Minor note: the V1
DeleteCustomRecipe(Lines 80–83) still returnsnilon a no-op delete, so any remaining V1 callers can't reproduce the same 404 semantics. If V1 isn't fully deprecated yet, consider aligning it to avoid divergent behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/dao/recipe.go` around lines 86 - 96, The V1 delete function DeleteCustomRecipe currently returns nil on no-op deletes which diverges from DeleteCustomRecipeV2; update DeleteCustomRecipe to check the gorm result.RowsAffected after the Delete call and return gorm.ErrRecordNotFound when RowsAffected == 0 (while still returning result.Error if set), mirroring the behavior in DeleteCustomRecipeV2 so callers of DeleteCustomRecipe can map missing records to HTTP 404 consistently.internal/craft/content_processors.go (1)
191-202: Heads-up: switching the hash function silently invalidates all existing cached craft results.Since
cacheKeyis derived from this hash, every previously cachedcleanup/fulltext/fulltext-plus(and, per the PR summary, LLM/translate) entry keyed with the MD5 scheme is now unreachable. On the first run after deploy all articles will be re-processed, which is costly for the LLM-backed paths (tokens, rate limits, latency).If that's acceptable as a one-time warmup, no action needed. Otherwise consider prefixing the new keys with a version tag so you can roll forward/back cleanly, and/or staggering rollout to smooth out the LLM concurrency spike.
Optional: versioned key
- return util.GetTextContentHash(content), nil + return "v2:" + util.GetTextContentHash(content), nil ... - return util.GetTextContentHash(uniqLinkStr), nil + return "v2:" + util.GetTextContentHash(uniqLinkStr), nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/craft/content_processors.go` around lines 191 - 202, The change to the cache key hash silently invalidates all existing cached craft results; update cacheKeyForCraftArticleContent and cacheKeyForCraftArticleLink to include a version prefix (e.g., "v2:" or similar configurable constant) when building the string passed to util.GetTextContentHash so you can roll keys forward/back without clobbering MD5-based caches—implement a small constant or env-driven VERSION_PREFIX used by both functions and include it in the hashed input (or as an unhashed prefix) to preserve backward/forward compatibility and allow staged rollout.web/admin/src/views/dashboard/observability/index.vue (1)
96-104: Prefer a named feature flag overfalse && …for the temporary disable.The
false &&short-circuit makes therecord.resource_type === 'topic'clause andgoToTopicDetaileffectively dead code, which tends to attract lint warnings and makes the "ready to re-enable" path easy to miss. A small named constant (or env/config flag) documents intent better and only needs a single edit to flip back on.♻️ Suggested shape
+ // TODO: flip to true once TopicFeed detail page is ready. + const TOPIC_FEED_DETAIL_ENABLED = false; @@ - <!-- TopicFeed 功能当前仍在开发完善中，先隐藏详情入口；待功能 ready 后再重新开放。 --> <a-button - v-if="false && record.resource_type === 'topic'" + v-if="TOPIC_FEED_DETAIL_ENABLED && record.resource_type === 'topic'" type="text" size="small" `@click`="goToTopicDetail(record.resource_id)" >Also applies to: 380-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin/src/views/dashboard/observability/index.vue` around lines 96 - 104, Replace the hard-coded short-circuit v-if ("false && record.resource_type === 'topic'") with a named feature flag (e.g., FEATURE_TOPIC_FEED) defined on the component (data/computed or imported config/env), then change the template to v-if="FEATURE_TOPIC_FEED && record.resource_type === 'topic'"; ensure the flag is false by default and documented where declared so flipping it (to true) re-enables the TopicFeed button and restores goToTopicDetail and the resource_type check; apply the same replacement for the other identical occurrence that currently uses "false && …".doc-site/src/content/docs/zh-tw/guides/advanced/customization.md (1)
76-76: Minor: Taiwan-style wording for the newLOG_LEVELline."日誌級別" / "後端應用" lean toward Mainland usage; Taiwan docs more typically use "日誌等級" (or "記錄層級") and "後端應用程式". Suggest a small polish for consistency with the rest of the zh-tw docs.
✏️ Suggested wording
-- **LOG_LEVEL**: (可選) 後端應用的日誌級別 (例如 `info`, `debug`, `trace`)。覆蓋 `ENV` 設定的預設級別。 +- **LOG_LEVEL**: (可選) 後端應用程式的日誌等級（例如 `info`、`debug`、`trace`）。覆蓋 `ENV` 設定的預設等級。As per coding guidelines: "For Traditional Chinese (zh-tw) documentation, use Taiwan-specific technical terms (e.g., 介面, 資料, 軟體)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc-site/src/content/docs/zh-tw/guides/advanced/customization.md` at line 76, The LOG_LEVEL description uses Mainland phrasing ("日誌級別", "後端應用"); update the string for zh-tw consistency by replacing "日誌級別" with "日誌等級" (or "記錄層級") and "後端應用" with "後端應用程式" in the LOG_LEVEL line so it reads e.g. "**LOG_LEVEL**: (可選) 後端應用程式的日誌等級 (例如 `info`, `debug`, `trace`)。覆蓋 `ENV` 設定的預設級別。", matching the zh-tw terminology used elsewhere.web/admin/src/router/routes/modules/worktable.ts (1)
74-78: Path rename is a breaking change for existing bookmarks; component folder still uses the old name.Two observations on the rename from
curl-to-rss→json-to-rss:
- Users who have bookmarked or deep-linked
/worktable/curl-to-rsswill now hit a 404. Consider adding a redirect route from the old path to preserve existing links.- The component is still loaded from
@/views/dashboard/curl_to_rss/curl_to_rss.vue. For consistency with the new naming and to avoid grep/search confusion, consider renaming the folder/file tojson_to_rss/json_to_rss.vuein a follow-up (the AI summary also indicates i18n namespaces moved fromcurlToRss→jsonToRss).🔧 Optional redirect example
{ path: 'json-to-rss', name: 'JsonToRss', component: () => import('@/views/dashboard/curl_to_rss/curl_to_rss.vue'), meta: { locale: 'menu.jsonToRss', requiresAuth: true, }, }, + { + path: 'curl-to-rss', + name: 'CurlToRssRedirect', + redirect: '/worktable/json-to-rss', + meta: { hideInMenu: true, requiresAuth: true }, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin/src/router/routes/modules/worktable.ts` around lines 74 - 78, The route rename from 'curl-to-rss' to 'json-to-rss' breaks existing bookmarks; add a new route entry that matches the old path 'curl-to-rss' and redirects (via the router's redirect option) to the route named 'JsonToRss' (or path '/worktable/json-to-rss') so old links resolve; also, as a follow-up, rename the component import and folder from '@/views/dashboard/curl_to_rss/curl_to_rss.vue' to '@/views/dashboard/json_to_rss/json_to_rss.vue' and update the dynamic import in the 'JsonToRss' route to the new path to keep names consistent.README.md (1)
22-22: Minor inconsistency between EN and ZH descriptions.The English line says "HTML/JSON/Search to RSS" without mentioning Curl, while the Chinese counterpart on Line 26 explicitly notes "JSON API (Curl)". For user-facing clarity, consider aligning both descriptions so that English readers also learn that Curl statement import is supported under the JSON-to-RSS path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 22, Update the English description sentence that currently reads "HTML/JSON/Search to RSS" to explicitly mention cURL support like the Chinese version; change it to "HTML/JSON (JSON API via cURL)/Search to RSS" or similar so it matches the Chinese "JSON API (Curl)" phrasing and makes the JSON-to-RSS cURL import capability clear.internal/engine/topic.go (1)
79-101: Double timestamp application is acceptable but redundant.
applyTopicFeedTimestampsis invoked once onmergedFeedand then again onprocessedFeedafter the aggregator runs. Since the aggregator may reorder/limit/deduplicate articles, recomputing after aggregation is correct; the initial call onmergedFeedis only useful when no aggregator is configured. This is fine as-is, but if the aggregator path is hot, you can skip the pre-aggregation call whent.Aggregator != nil.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/engine/topic.go` around lines 79 - 101, The code currently calls applyTopicFeedTimestamps(mergedFeed) before branching to run t.Aggregator, causing redundant timestamp work when an aggregator exists; change the flow so applyTopicFeedTimestamps is only called on mergedFeed when t.Aggregator is nil, and keep the call on processedFeed after t.Aggregator.Process(ctx, mergedFeed) so timestamps are recomputed post-aggregation; locate the calls to applyTopicFeedTimestamps, the t.Aggregator nil check, and variables mergedFeed/processedFeed to implement this conditional move.web/admin/src/views/dashboard/curl_to_rss/curl_to_rss.vue (1)
1-6: Consider renaming the component file to match the feature rename.The feature has been renamed to "JSON to RSS" (i18n keys:
menu.jsonToRss,jsonToRss.description, etc.), but the file path is stillweb/admin/src/views/dashboard/curl_to_rss/curl_to_rss.vue. This creates a divergence between directory/file naming and user-facing naming. Rename the directory and file tojson_to_rss/json_to_rss.vueand update the route import inweb/admin/src/router/routes/modules/worktable.ts:76.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin/src/views/dashboard/curl_to_rss/curl_to_rss.vue` around lines 1 - 6, The component filename and directory still use curl_to_rss while the feature and i18n keys use jsonToRss; rename the component directory and file from curl_to_rss/curl_to_rss.vue to json_to_rss/json_to_rss.vue and update any imports that reference the old name (notably the route import in the worktable route module that imports curl_to_rss) to import json_to_rss instead; ensure the component registration/export name inside the Vue file (if present) stays consistent with the new filename.web/admin/src/views/dashboard/feed_viewer/feed_view_container.vue (1)
85-91: Optional: simplify image value composition.The current expression relies on operator precedence (
||before?:) and repeats the optional chain four times. A filter-based form is easier to read and yields the same output (title/url joined by a single space, trimmed, or empty when both are missing).♻️ Proposed refactor
- { - label: 'image', - value: - data.image?.url || data.image?.title - ? `${data.image?.title || ''} ${data.image?.url || ''}`.trim() - : '', - }, + { + label: 'image', + value: [data.image?.title, data.image?.url] + .filter(Boolean) + .join(' '), + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin/src/views/dashboard/feed_viewer/feed_view_container.vue` around lines 85 - 91, Replace the complex conditional expression used for the object with label 'image' and its value by building an array from data.image?.title and data.image?.url, filtering out falsy values, joining them with a single space, and trimming the result; this removes repeated optional chaining (data.image?.title/data.image?.url) and the nested ?:/|| logic while keeping the same empty-string fallback when both are missing.internal/controller/feed_viewer.go (3)
192-215: Reduce cyclomatic complexity (Codacy: 10 > 8).Extracting the IP-safety check into a helper both addresses the static-analysis warning and makes the policy easy to reuse/test. Pairs nicely with the broader SSRF-rule expansion suggested above.
♻️ Proposed refactor
+func isDisallowedIP(ip net.IP) bool { + return ip.IsLoopback() || ip.IsPrivate() || ip.IsUnspecified() || + ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || + ip.IsMulticast() || ip.IsInterfaceLocalMulticast() +} + func validateFeedViewerURL(rawURL string) error { parsedURL, err := url.Parse(rawURL) - if err != nil || parsedURL == nil { - return errors.New("Please enter a valid http(s) feed URL") - } - if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { - return errors.New("Please enter a valid http(s) feed URL") - } - if parsedURL.Hostname() == "" { + if err != nil || parsedURL == nil || + (parsedURL.Scheme != "http" && parsedURL.Scheme != "https") || + parsedURL.Hostname() == "" { return errors.New("Please enter a valid http(s) feed URL") } ips, err := net.LookupIP(parsedURL.Hostname()) if err != nil { return fmt.Errorf("Unable to resolve this URL: %w", err) } for _, ip := range ips { - if ip.IsLoopback() || ip.IsPrivate() { - return fmt.Errorf("Access to private IP %s is forbidden", ip.String()) + if isDisallowedIP(ip) { + return fmt.Errorf("Access to non-public IP %s is forbidden", ip.String()) } } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/feed_viewer.go` around lines 192 - 215, The validateFeedViewerURL function has high cyclomatic complexity due to inline IP-safety checks; extract the IP resolution and validation into a helper (e.g., hostIPsAreUnsafe or isHostPrivateOrLoopback) and call it from validateFeedViewerURL. Move the net.LookupIP call and the loop that checks ip.IsLoopback() || ip.IsPrivate() into that helper so validateFeedViewerURL only parses/validates the URL and delegates host safety to the new function, returning the helper's error when unsafe.
217-235: Fragile error classification via substring matching.Classifying errors by
strings.Contains(msg, "http status not ok:")etc. silently breaks the moment any upstream error-message wording changes (e.g. a dependency bump ingofeed,source.Fetch, or the browserless client). Consider returning typed sentinel errors from the fetch/parse layers and usingerrors.Is/errors.Ashere. This also eliminates the need to strip the"all items failed to process. last error: "prefix on line 219.Not blocking — current behavior is reasonable — but worth tracking, especially since the UI's user-facing messages depend on this mapping being correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/feed_viewer.go` around lines 217 - 235, The classifyFeedViewerError function currently relies on fragile substring matches (and trims the "all items failed..." prefix) which breaks when upstream error text changes; instead, introduce and return typed sentinel or wrapped errors from the fetch/parse layers (e.g., from source.Fetch, the browserless client, and gofeed parsing) such as ErrHTTPStatus, ErrFetchFailed, ErrParseFailed, ErrInvalidCraft, etc., then change classifyFeedViewerError to use errors.Is / errors.As to detect those concrete error types and map them to the appropriate status/message (remove the strings.TrimPrefix usage and match on error values rather than message substrings). Ensure the new error types are exported or package-internal where classifyFeedViewerError can reference them and update call sites to wrap/return those sentinel errors so the mapping remains stable.
100-102: Extract "proxy" to a named constant for discoverability and maintainability.
req.CraftName == "proxy"is a shared sentinel appearing across the codebase (controller, craft/entry.go, craft/runtime.go, and tests) to represent "no processing" or passthrough. Define this once as a typed constant ininternal/constant/(following the pattern ofProcessorType) and reuse it to make the contract explicit and refactor-safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/feed_viewer.go` around lines 100 - 102, Extract the literal "proxy" into a typed constant (e.g., ProxyCraftName string) under internal/constant/ following the existing ProcessorType pattern, then replace direct string comparisons of req.CraftName == "proxy" in feed_viewer.go (and the other occurrences in controller, craft/entry.go, craft/runtime.go, and tests) to compare against constantpackage.ProxyCraftName; ensure the constant is exported and import the internal/constant package where needed so all sentinel checks use the new named constant for clarity and maintainability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf936f48-5c50-40ec-8046-1b526208117f
⛔ Files ignored due to path filters (1)
web/admin/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (82)
.env.example.github/workflows/docker-publish.yml.github/workflows/lint.ymlREADME.mddoc-site/src/content/docs/en/guides/advanced/customization.mddoc-site/src/content/docs/en/guides/advanced/html-to-rss.mddoc-site/src/content/docs/en/guides/advanced/json-to-rss.mddoc-site/src/content/docs/en/guides/advanced/tools.mddoc-site/src/content/docs/en/guides/start/concepts.mddoc-site/src/content/docs/zh-tw/guides/advanced/customization.mddoc-site/src/content/docs/zh-tw/guides/advanced/html-to-rss.mddoc-site/src/content/docs/zh-tw/guides/advanced/json-to-rss.mddoc-site/src/content/docs/zh-tw/guides/advanced/tools.mddoc-site/src/content/docs/zh-tw/guides/start/concepts.mddoc-site/src/content/docs/zh/guides/advanced/customization.mddoc-site/src/content/docs/zh/guides/advanced/html-to-rss.mddoc-site/src/content/docs/zh/guides/advanced/json-to-rss.mddoc-site/src/content/docs/zh/guides/advanced/tools.mddoc-site/src/content/docs/zh/guides/start/concepts.mdinternal/adapter/llm.gointernal/config/source_config.gointernal/controller/craft_flow.gointernal/controller/curl_to_rss.gointernal/controller/custom_recipe.gointernal/controller/feed_viewer.gointernal/controller/html_to_rss.gointernal/craft/benchmark_test.gointernal/craft/content_processors.gointernal/craft/guid.gointernal/craft/llm_processors.gointernal/craft/option.gointernal/craft/option_fetch_test.gointernal/craft/runtime.gointernal/craft/runtime_test.gointernal/craft/test_redis_helper_test.gointernal/craft/translate.gointernal/dao/migrate.gointernal/dao/recipe.gointernal/engine/topic.gointernal/engine/topic_test.gointernal/feedruntime/builder.gointernal/feedruntime/builder_test.gointernal/router/registry.gointernal/source/fetcher/http_fetcher.gointernal/source/fetcher/http_fetcher_test.gointernal/source/fetcher/provider/litellm.gointernal/source/fetcher/provider/litellm_test.gointernal/source/fetcher/provider/searxng.gointernal/source/fetcher/provider/searxng_test.gointernal/source/html.gointernal/source/json.gointernal/source/parser/json_parser.gointernal/source/parser/json_parser_test.gointernal/source/rss.gointernal/source/source_factory_test.gointernal/util/browserless.gointernal/util/env_var.gointernal/util/env_var_test.gointernal/util/hash.goproposal/future/html_token_optimization.mdproposal/inbox_source_design.mdweb/admin/index.htmlweb/admin/src/api/feed_viewer.tsweb/admin/src/locale/en-US.tsweb/admin/src/locale/en-US/curlToRss.tsweb/admin/src/locale/en-US/health.tsweb/admin/src/locale/en-US/jsonToRss.tsweb/admin/src/locale/en-US/menu.tsweb/admin/src/locale/zh-CN.tsweb/admin/src/locale/zh-CN/curlToRss.tsweb/admin/src/locale/zh-CN/health.tsweb/admin/src/locale/zh-CN/jsonToRss.tsweb/admin/src/locale/zh-CN/menu.tsweb/admin/src/router/routes/modules/worktable.tsweb/admin/src/views/dashboard/curl_to_rss/curl_to_rss.vueweb/admin/src/views/dashboard/custom_recipe/custom_recipe.vueweb/admin/src/views/dashboard/feed_compare/feed_compare.vueweb/admin/src/views/dashboard/feed_viewer/feed_view_container.vueweb/admin/src/views/dashboard/feed_viewer/feed_viewer.vueweb/admin/src/views/dashboard/health/index.vueweb/admin/src/views/dashboard/html_to_rss/utils/selector.tsweb/admin/src/views/dashboard/observability/index.vue
💤 Files with no reviewable changes (3)
- internal/feedruntime/builder.go
- web/admin/src/locale/zh-CN/curlToRss.ts
- web/admin/src/locale/en-US/curlToRss.ts
| version: latest | ||
|
|
||
| - uses: pnpm/action-setup@v3 | ||
| - uses: pnpm/action-setup@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
pnpm/action-setup v6 breaking changes migration guide
💡 Result:
pnpm/action-setup v6 introduces several breaking changes and known issues compared to v5. There is no official migration guide in the repository README or releases, but changes can be inferred from the v4...v6 compare view, release notes, and open issues. Key breaking changes: - Replaces bundled pnpm binary with npm + lockfile bootstrap using pnpm v11 bootstrap (commit 58e6119). This changes installation mechanism and may cause PATH shadowing issues. - Requires Node.js 24+ (feat!: run the action on Node.js 24, commit fc06bc1). - Updated version detection logic: now reads from devEngines.packageManager in addition to packageManager field, and skips installation if packageManager is set (allowing pnpm self-update). - Removed dist/pnpm.cjs and dist/worker.js (large binary bundles). - Examples updated to actions/checkout@v6 and pnpm version examples to 10.x. Known issues in early v6 releases (fixed in later patches like v6.0.2+): - Does not respect specified version (e.g., version: 10 installs v11 bootstrap instead): Issues #225, #227. Fixed in v6.0.2 by swapping PATH add order (PR #230). - Modifies pnpm-lock.yaml unexpectedly when used with actions/setup-node cache: "pnpm" cache (Issue #226). Causes ERR_PNPM_LOCKFILE_CONFIG_MISMATCH or invalid YAML with multiple documents (Issue #228). pnpm v11 writes extra YAML document for packageManagerDependencies; fixed if packageManager is set in package.json or using latest pnpm v10+. - pnpm version commands like pnpm version patch --no-git-tag-version fail due to v11 bootstrap lacking npm passthrough. Migration steps: 1. Update to latest v6.x (e.g., v6.0.3 as of 2026-04-21): uses: pnpm/action-setup@v6 (pins to latest v6). Or pin to fixed commit: uses: pnpm/action-setup@2e223e0 # v6 (pre-fix). 2. Ensure Node.js >=24 via actions/setup-node. 3. Explicitly specify version: 10.x (latest stable) if not using packageManager field, as v6 defaults to v11 bootstrap. 4. If using packageManager: "pnpm@X.Y.Z" in package.json, omit version input - pnpm will self-update to match. 5. For cache issues: Set packageManager in package.json to avoid lockfile modifications, update lockfile with pnpm install, use latest pnpm v10+, or run pnpm install without --frozen-lockfile initially. Avoid auto-upgrades via Dependabot until issues resolved. 6. Test workflows thoroughly, especially with --frozen-lockfile, nx, or version commands. 7. If issues persist, pin to v5: uses: pnpm/action-setup@v5. Official releases: v6.0.0 (2026-04-10: pnpm v11 support), v6.0.2 (2026-04-18: PATH fix), v6.0.3 (2026-04-21: pnpm v11.0.0-rc.5).
Citations:
- 1: action-setup@v6 does not take the requested pnpm version into account pnpm/action-setup#225
- 2: pnpm/action-setup@v4...v6
- 3: v6.0.0 actions/setup-node PNPM cache modifies pnpm-lock.yaml pnpm/action-setup#226
- 4: Bump pnpm/action-setup from 4 to 6 actions/setup-node#1528
- 5: https://github.com/pnpm/action-setup/releases
- 6: https://github.com/pnpm/action-setup/releases/tag/v6.0.2
- 7: https://github.com/pnpm/action-setup/releases/tag/v6.0.3
- 8: action-setup@v6 does not install pnpm version specified in
package_json_filepnpm/action-setup#227 - 9: ci: bump pnpm/action-setup from 5.0.0 to 6.0.0 eslint/eslint#20763
- 10: action-setup v6 renders invalid YAML file pnpm/action-setup#228
Upgrade to pnpm/action-setup@v6 introduces breaking changes; verify compatibility before merging.
v6 requires Node.js 24+ and changes the pnpm installation mechanism. Early v6 releases had a bug where the version input was ignored (installing v11 instead of v9), fixed in v6.0.2+. Ensure you:
- Pin to v6.0.2 or later
- Update Node.js requirement to 24+ (via actions/setup-node or runner)
- Explicitly set
version: 10.x(latest stable) instead of 9, since v6 defaults to v11 bootstrap when no version is specified - Test the workflow to verify no cache conflicts occur with pnpm-lock.yaml
Alternatively, downgrade to v5 if these changes are not feasible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/lint.yml at line 29, The workflow change replacing uses:
pnpm/action-setup@v6 is breaking; update the action usage to pin a v6 bugfix
(>=v6.0.2) and explicitly set the action's version input (e.g., version: 10.x)
instead of relying on defaults, and ensure the Node.js runtime requirement is
raised to 24+ via your actions/setup-node step (or runner) to match pnpm v6;
after making these changes run the lint workflow to verify pnpm-lock.yaml
cache/restore behavior and conflicts, or revert to pnpm/action-setup@v5 if you
cannot meet the Node 24+ requirement.
|
|
||
| :::note | ||
| This tool is designed for HTML pages. If you need to process a JSON API, use the [CURL to RSS](/en/guides/advanced/curl-to-rss/) instead. | ||
| This tool is designed for HTML pages. If you need to process a JSON API, use the [JSON to RSS](/en/guides/advanced/json-to-rss/) instead. |
There was a problem hiding this comment.
Use a relative link and tidy the phrasing.
Two small issues on this line:
- Same-locale link should be relative. As per coding guidelines: "Prefer relative links (e.g., ../advanced/customization) when linking between documents within the same language tree" and "Use absolute links starting with the locale … only when necessary, such as in the index page or cross-locale references".
- Minor grammar: "use the [JSON to RSS] instead" reads awkwardly — either drop "the" or add "guide" after the link.
📝 Proposed fix
-This tool is designed for HTML pages. If you need to process a JSON API, use the [JSON to RSS](/en/guides/advanced/json-to-rss/) instead.
+This tool is designed for HTML pages. If you need to process a JSON API, use the [JSON to RSS](./json-to-rss/) guide instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc-site/src/content/docs/en/guides/advanced/html-to-rss.md` at line 14, The
sentence uses an absolute same-locale link and awkward phrasing; replace "This
tool is designed for HTML pages. If you need to process a JSON API, use the
[JSON to RSS](/en/guides/advanced/json-to-rss/) instead." with a version that
uses a relative link and cleaner wording, e.g. "This tool is designed for HTML
pages. If you need to process a JSON API, use the [JSON to RSS
guide](../advanced/json-to-rss/) instead." — update the line containing that
sentence to use "../advanced/json-to-rss/" and either add "guide" after the link
or remove "the" to improve grammar.
|
|
||
| - **清理標題空白字元**：`{{ .Fields.Title | trimSpace }}` | ||
| - **拼接完整連結**：`https://example.com/article/{{ .Item.id }}` | ||
| - **移除特定前缀**：`{{ .Fields.Description | trim "Prefix: " }}` |
There was a problem hiding this comment.
简体字误用：前缀 应为 前綴。
在 zh-tw 繁體中文文件中，「缀」是簡體字，繁體應為「綴」。建議改為「前綴」。
📝 建議修改
-- **移除特定前缀**：`{{ .Fields.Description | trim "Prefix: " }}`
+- **移除特定前綴**：`{{ .Fields.Description | trim "Prefix: " }}`As per coding guidelines: "For Traditional Chinese (zh-tw) documentation, use Taiwan-specific technical terms".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **移除特定前缀**：`{{ .Fields.Description | trim "Prefix: " }}` | |
| - **移除特定前綴**：`{{ .Fields.Description | trim "Prefix: " }}` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc-site/src/content/docs/zh-tw/guides/advanced/json-to-rss.md` at line 72,
The heading line "**移除特定前缀**：`{{ .Fields.Description | trim \"Prefix: \" }}`"
uses the Simplified Chinese character "前缀"; update this to the Traditional
Chinese "前綴" so the heading reads "**移除特定前綴**：`{{ .Fields.Description | trim
\"Prefix: \" }}`" to comply with zh-tw terminology; ensure any other occurrences
of "前缀" in the same document are also replaced with "前綴".
Comment on lines
+68
to
+69
| - **FC_HTTP_USER_AGENT_FEED**: （可选）feed 类外部请求的默认 `User-Agent`，例如抓取 RSS/XML 资源时使用。搜索提供方请求目前也临时归入这一规则。 | ||
| - **FC_HTTP_USER_AGENT_HTML**: （可选）HTML 页面抓取的默认 `User-Agent`，例如全文提取和 HTML 转 RSS 工具使用。**注意：** 如果该值包含空格或括号，必须使用引号括起来。 |
There was a problem hiding this comment.
文档不一致：FC_HTTP_USER_AGENT_FEED 缺少关于引号的说明。
第 69 行为 FC_HTTP_USER_AGENT_HTML 添加了 "如果该值包含空格或括号，必须使用引号括起来" 的提示，但第 68 行的 FC_HTTP_USER_AGENT_FEED 没有相同说明，而该变量同样会被用户设置为包含空格/括号的典型 UA 字符串（例如 Mozilla/5.0 (...)）。建议在第 68 行补上相同的引号说明，以保持一致性。
📝 建议修改
-- **FC_HTTP_USER_AGENT_FEED**: （可选）feed 类外部请求的默认 `User-Agent`，例如抓取 RSS/XML 资源时使用。搜索提供方请求目前也临时归入这一规则。
+- **FC_HTTP_USER_AGENT_FEED**: （可选）feed 类外部请求的默认 `User-Agent`，例如抓取 RSS/XML 资源时使用。搜索提供方请求目前也临时归入这一规则。**注意：** 如果该值包含空格或括号，必须使用引号括起来。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc-site/src/content/docs/zh/guides/advanced/customization.md` around lines
68 - 69, Update the documentation to be consistent: add the same note about
quoting to the FC_HTTP_USER_AGENT_FEED entry that already exists for
FC_HTTP_USER_AGENT_HTML — explicitly state that if the value contains spaces or
parentheses it must be wrapped in quotes; keep wording parallel to the
FC_HTTP_USER_AGENT_HTML line so both entries (FC_HTTP_USER_AGENT_FEED and
FC_HTTP_USER_AGENT_HTML) convey the same quoting requirement for User-Agent
strings.
|
|
||
| finalPrompt := fmt.Sprintf("%s \n```\n%s\n```", prompt, processedContext) | ||
| cacheKey := fmt.Sprintf("llm_call_%s", util.GetMD5Hash(finalPrompt)) | ||
| cacheKey := fmt.Sprintf("llm_call_%s", util.GetTextContentHash(finalPrompt)) |
There was a problem hiding this comment.
Cache invalidation after the MD5 → GetTextContentHash migration
Switching the hash function changes every cacheKey for the same finalPrompt, so all previously cached LLM responses will be effectively orphaned. Expect a cold cache (and a temporary spike in LLM calls / latency / cost) on first deployment. Consider whether the cache backend has a TTL/eviction policy to reclaim the old entries, or add a one-time cleanup for keys matching llm_call_*.
The same concern applies to the other MD5Hash → GetTextContentHash migrations in internal/craft/* cache keys — noting it once here as the root cause.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/adapter/llm.go` at line 16, Switching from MD5Hash to
util.GetTextContentHash changed the cache key format (cacheKey :=
fmt.Sprintf("llm_call_%s", util.GetTextContentHash(finalPrompt))) which orphaned
previous entries; to fix, add a backward-compatibility lookup or one-time
migration: when resolving cache for llm calls in the function that builds
cacheKey, attempt a fallback read using the old MD5-based key (e.g.,
llm_call_<oldMD5>) before treating as a miss, and/or implement a maintenance
task that scans the cache backend for keys matching "llm_call_*" and deletes or
migrates them (ensure safe, idempotent behavior and respect TTLs/eviction
policies).
Comment on lines
92
to
95
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("http get failed: %w", err) | ||
| return nil, &fetchError{err: fmt.Errorf("http get failed: %w", err), retryable: true} | ||
| } |
There was a problem hiding this comment.
Set an explicit HTTP client timeout.
http.DefaultClient has Timeout: 0 (no timeout). If a caller passes a context.Background() or a long-lived context, a hung/slow upstream can block the request (and its retries) indefinitely. Given this is the shared default client, the impact is process-wide.
Use a dedicated client with an explicit timeout so each attempt is bounded even when the context has no deadline:
🔧 Proposed fix
+var httpClient = &http.Client{Timeout: 30 * time.Second}
+
func (f *HttpFetcher) doRequest(ctx context.Context, profile requestProfile) ([]byte, error) {
...
- resp, err := http.DefaultClient.Do(req)
+ resp, err := httpClient.Do(req)Consider making the timeout configurable via HttpFetcherConfig (similar to the browserless path at Line 36) for long-tail feeds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/source/fetcher/http_fetcher.go` around lines 92 - 95, The code uses
http.DefaultClient.Do(req) which has no timeout; replace usage in the HTTP fetch
path (the call at resp, err := http.DefaultClient.Do(req)) with a dedicated
*http.Client having an explicit Timeout (e.g. from a new
HttpFetcherConfig.HttpClientTimeout field or a sensible default) and use that
client to perform the request so each attempt is bounded; ensure the new client
is constructed/owned by the HttpFetcher (or created per request if desired) and
keep the same error handling (wrapping errors into fetchError) so behavior is
unchanged aside from enforcing the timeout.
Comment on lines
+299
to
+306
| ### 9.2 内容读取（公开访问） | ||
|
|
||
| | 方法 | 路径 | 说明 | | ||
| | ------- | ------------------------------------------------- | --------------------------------- | | ||
| | **GET** | `<feedcraft-site-base-url>/:inbox_id/:article_id` | 返回对应文章的 `content` 字段内容 | | ||
|
|
||
| 该路由中的 `article_id` 对应 `InboxItem.ItemID`。如果某条数据在 POST 写入时未提供 `url`，系统会自动将 `url` 回填为这个内容访问地址。 | ||
|
|
There was a problem hiding this comment.
Consider access control and security implications for the public content endpoint.
This section introduces a publicly accessible GET endpoint with no authentication. This could expose sensitive data if:
- Inbox items contain confidential information
- Callers use predictable
ItemIDvalues instead of UUIDs, enabling enumeration - No per-inbox access control exists
Consider:
- Adding optional authentication/authorization (e.g., read tokens per inbox)
- Documenting the security trade-offs in the design
- Recommending UUID-only
ItemIDfor public-facing inboxes - Adding a configuration flag to enable/disable public access per inbox
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@proposal/inbox_source_design.md` around lines 299 - 306, The public GET route
`<feedcraft-site-base-url>/:inbox_id/:article_id` (which returns
InboxItem.content and where article_id maps to InboxItem.ItemID) lacks access
controls and can lead to data exposure; update the design and implementation to:
add optional per-inbox read tokens or auth (document an Authorization/read-token
header) and enforce per-inbox configuration (e.g., inbox.public_read boolean) to
allow toggling public access, mandate UUID-formatted ItemID for any inbox with
public_read=true (reject/convert predictable IDs on POST when writing URL
fallback), and add documentation notes describing the security trade-offs and
recommended defaults.
| 'health.missing': '缺失', | ||
| 'health.noData': '暂无分析数据，请点击分析按钮开始。', | ||
| 'health.fetchError': '获取依赖健康状态失败', | ||
| 'health.missingCrafts': '缺失的 Crafts', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP "missingCrafts" web/admin/src/localeRepository: Colin-XKL/FeedCraft
Length of output: 225
Add missing health.missingCrafts key to zh-TW locale.
The key exists in zh-CN and en-US, but is missing from web/admin/src/locale/zh-TW/health.ts. Without it, the zh-TW locale will display the raw key instead of a translated string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/admin/src/locale/zh-CN/health.ts` at line 9, Add the missing localization
key health.missingCrafts to the zh-TW health locale file
(web/admin/src/locale/zh-TW/health.ts) so it matches zh-CN and en-US; insert the
key with an appropriate Traditional Chinese translation (for example: "缺少的
Crafts") to prevent the raw key from being shown in the UI.
| 'jsonToRss.placeholder.recipeId': 'my-json-feed', | ||
| 'jsonToRss.placeholder.internalDesc': '关于此配方的备注', | ||
| 'jsonToRss.placeholder.url': 'https://api.example.com/v1/posts', | ||
| 'jsonToRss.placeholder.body': "{ 'foo': 'bar' }", |
There was a problem hiding this comment.
Invalid JSON shown as body placeholder.
{ 'foo': 'bar' } is not valid JSON (keys/strings must be double-quoted). Since this field is the HTTP request body input for a JSON source, the example should itself be valid JSON.
✏️ Proposed fix
- 'jsonToRss.placeholder.body': "{ 'foo': 'bar' }",
+ 'jsonToRss.placeholder.body': '{ "foo": "bar" }',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 'jsonToRss.placeholder.body': "{ 'foo': 'bar' }", | |
| 'jsonToRss.placeholder.body': '{ "foo": "bar" }', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/admin/src/locale/zh-CN/jsonToRss.ts` at line 85, The placeholder for the
JSON request body under the key 'jsonToRss.placeholder.body' uses invalid JSON
({ 'foo': 'bar' }); update that value to a valid JSON example with double-quoted
keys/strings (e.g., {"foo":"bar"} or a nicely formatted string) so the HTTP
request body input shows valid JSON.
| } | ||
|
|
||
| return path.join(' > '); | ||
| return path.join(' '); |
There was a problem hiding this comment.
Confirm the switch from child combinator to descendant combinator is intentional.
Joining path segments with " " instead of " > " changes the generated selector semantics from strict parent→child to any descendant relationship. Combined with the :nth-of-type(n) branch above, this makes the resulting selector strictly more permissive — any element matching the tag/class at any depth and being the nth-of-type within its own parent will match, even under intermediate wrappers that weren’t in the original path.
For HTML-to-RSS item selection this may be desirable (more robust to wrapper changes), but it can also cause false matches inside nested lists/cards that share tag names (e.g., li nested within another li). Please confirm this is intentional and covered by tests against representative pages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/admin/src/views/dashboard/html_to_rss/utils/selector.ts` at line 78, The
change from a child combinator to a descendant combinator is likely coming from
the return that uses path.join(' ') (replacing the previous ' > ' join) and it
interacts with the :nth-of-type(n) branch to make selectors more permissive;
confirm whether this is intentional and either revert path.join(' ') back to
path.join(' > ') in the selector builder (e.g., where path.join(' ') is
returned) to preserve strict parent→child semantics or, if the looser descendant
behavior is desired, add a clear inline comment documenting the intent and add
unit/integration tests exercising nested structures (e.g., nested li inside li,
nested cards) to ensure no false positives.
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
2 participants
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 by Sourcery
Rename and generalize the CURL-to-RSS functionality into a JSON-to-RSS tool, introduce centralized HTTP user-agent and retry behavior for different fetch purposes, add a backend feed preview API used by the UI feed viewer and comparator, tighten deletion/error handling and hashing semantics, and expand documentation and tests around new behavior and observability tools.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation: