fix(plantask): resolve listed task file paths in TaskList and TaskCreate#933
Open
GaosCode wants to merge 1 commit intocloudwego:mainfrom
Open
fix(plantask): resolve listed task file paths in TaskList and TaskCreate#933GaosCode wants to merge 1 commit intocloudwego:mainfrom
GaosCode wants to merge 1 commit intocloudwego:mainfrom
Conversation
Author
|
Hi, this PR should be ready for review. It fixes #932 and the local validation passed on my side. The remaining blocker seems to be maintainer review / workflow approval. When someone has time, could you please take a look? Thank you! |
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.
Summary
This PR fixes a path handling bug in
plantaskwhenBackend.LsInforeturns file names or relative paths instead of full file paths.It updates both
TaskListandTaskCreateto resolve listed entries againstbaseDirbefore reading them, and aligns theplantasktest backend with the documented backend contract so the failing scenario is actually covered by tests.Problem
plantaskcurrently assumes thatBackend.LsInforeturns full file paths.In practice, the backend contract allows
FileInfo.Pathto be a file name or a relative path. Because of that,TaskListmay fail when it reads a listed task file directly:The same assumption also exists in
TaskCreatewhen it scansLsInforesults and reads.highwatermark.Root Cause
TaskListlists files frombaseDir, but passesfile.Pathdirectly toReadwithout resolving it first.That only works when
LsInforeturns absolute paths. If the backend returns1.json,TaskListdoes not join it withbaseDirbefore callingRead.TaskCreatehas the same issue when handling.highwatermark.Solution
This PR introduces a shared helper for resolving listed file paths:
LsInfoalready returns an absolute pathbaseDirwhenLsInforeturns a file name or relative pathThe same logic is applied in both places:
TaskListwhen reading task filesTaskCreatewhen reading.highwatermarkTest Coverage
This PR also updates the
plantasktest backend to return basename-style paths fromLsInfo, which is closer to the documented backend behavior.That makes the tests cover the real failing scenario instead of masking it with full paths.
Validation
Validated locally with:
All passed.
Fixes #932