Repository navigation
fix: docs delete input validation, and same-second document list order - #529
Merged
Merged
Conversation
`libscope docs delete <id>` looked the document up with
`get-document {maxLength: 0}` to show its title, but get-document
requires a positive maxLength, so every delete failed with
"Invalid input for get-document: maxLength: Too small". Look the
document up without maxLength; getDocumentView reads the whole
content either way.
The CLI contract tests replace runOperation with a spy, so inputs were
never validated and the test asserted maxLength: 0. invoke() now checks
each call's input against the operation's schema.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
listDocuments sorted only by updated_at, which has one-second resolution, so documents updated in the same second came back oldest first and in an order that depended on timing. That made the lite test "gives the same results as LibScope.create" flaky on Node 20: when one database's inserts crossed a second boundary and the other's did not, the two lists differed. Break ties by rowid, newest insert first.
RobertLD
changed the title
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1.
docs deletealways failedlibscope docs delete <id>fails in 2.0.1:To show the title in the confirmation, the command calls
get-documentwithmaxLength: 0(src/cli/commands/docs.ts:173).get-documentrequiresmaxLengthto be a positive integer, so the lookup fails before anything is deleted.Fix: call
get-documentwithoutmaxLength.getDocumentViewreads the full content either way and only slices it, so this costs nothing.Tests: the CLI contract test replaces
runOperationwith a spy, so inputs were never validated, and it assertedmaxLength: 0(the failing input) as the expected call. The expectation now matches the fix, andinvoke()checks every operation call's input against the operation's own schema. Without the code fix, the docs delete test fails withget-document input: … "maxLength". All 94 CLI contract tests pass the new check, so no other command sends invalid input.2. Document list order within the same second
listDocumentssorted only byupdated_at, which has one-second resolution. Documents updated in the same second came back oldest first, in an order that depended on timing. That madelite.test.ts"gives the same results as LibScope.create" flaky: it failed on Node 20 in this PR's first CI run (received Vue, Angular, React; expected React, Vue, Angular), consistent with one database's inserts crossing a second boundary and the other's not. I inferred that cause from the failure; I did not reproduce the timing.Fix:
ORDER BY d.updated_at DESC, d.rowid DESC, newest insert first on ties.Tests: one new test in
documents.test.tssets the sameupdated_aton every document and expects newest insert first. It fails without the fix.lite.test.tsis unchanged.Verification
adda file, thendocs delete <id> --yesprints✓ Deleted "delete-me" (…), exit 0, and no documents remain.npm run lint(0 errors),format:check,typecheck,test:coverage: 2139 tests pass.