-
Notifications
You must be signed in to change notification settings - Fork 1
fix: private repository support and parser compatibility #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
URLs like 'https://github.com/user/repo/' (with trailing slash) would fail because the full_name became 'user/repo/' instead of 'user/repo'. Also added better error logging for GitHub API failures.
The git.pull() call was missing onAuth, causing it to fail silently for private repositories. Also added better logging for debugging.
unified v11+ has ESM/CJS compatibility issues when used with require(). Using remark() directly works correctly and handles parsing internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes critical issues with private repository support and improves parser compatibility for the Quartorium application. The changes address three main problems: URL parsing failures with trailing slashes, missing authentication for git pull operations on private repos, and ESM/CJS compatibility issues with the unified/remark parser.
- Fixed URL parsing to handle trailing slashes in repository URLs
- Added authentication (
onAuth) togit.pull()for private repository access - Migrated from
unified().use(remarkParse)toremark().parse()to resolve ESM/CJS compatibility issues
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| backend/src/core/quartoParser.js | Simplifies markdown parsing by switching to remark's built-in parser, fixing ESM/CJS compatibility issues |
| backend/src/api/repos.routes.js | Adds trailing slash handling in URL parsing, authentication for git pull operations, and improves error logging for debugging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Cleans up path, e.g., /user/repo.git -> user/repo | ||
| const full_name = url.pathname.slice(1).replace(/\.git$/, ''); | ||
| // Cleans up path, e.g., /user/repo.git/ -> user/repo | ||
| const full_name = url.pathname.slice(1).replace(/\/+$/, '').replace(/\.git$/, ''); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order of operations in the URL cleanup could leave a trailing slash in edge cases. For example, a URL ending in /.git would result in user/repo/ after .git is removed. Consider swapping the order:
const full_name = url.pathname.slice(1).replace(/\.git$/, '').replace(/\/+$/, '');This ensures trailing slashes are removed after the .git suffix is handled.
| const full_name = url.pathname.slice(1).replace(/\/+$/, '').replace(/\.git$/, ''); | |
| const full_name = url.pathname.slice(1).replace(/\.git$/, '').replace(/\/+$/, ''); |
| if (error.response) { | ||
| console.error("GitHub API error:", { | ||
| status: error.response.status, | ||
| message: error.response.data?.message, | ||
| documentation_url: error.response.data?.documentation_url | ||
| }); | ||
| } else { | ||
| console.error("Error adding repo:", error.message); | ||
| } | ||
| res.status(404).json({ error: 'Repository not found or you do not have access.' }); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All errors in the try-catch block return a 404 status code, but not all errors represent "not found" scenarios. For example, if the URL parsing fails (line 47) or if it's not a GitHub URL (line 48-49), a 400 Bad Request would be more appropriate. Consider differentiating error types:
} catch (error) {
if (error.response) {
console.error("GitHub API error:", {
status: error.response.status,
message: error.response.data?.message,
documentation_url: error.response.data?.documentation_url
});
res.status(404).json({ error: 'Repository not found or you do not have access.' });
} else {
console.error("Error adding repo:", error.message);
res.status(400).json({ error: error.message });
}
}| if (error.response) { | |
| console.error("GitHub API error:", { | |
| status: error.response.status, | |
| message: error.response.data?.message, | |
| documentation_url: error.response.data?.documentation_url | |
| }); | |
| } else { | |
| console.error("Error adding repo:", error.message); | |
| } | |
| res.status(404).json({ error: 'Repository not found or you do not have access.' }); | |
| if ( | |
| error instanceof TypeError || // Malformed URL | |
| error.message === 'Not a GitHub URL' | |
| ) { | |
| console.error("Bad request error adding repo:", error.message); | |
| return res.status(400).json({ error: error.message }); | |
| } | |
| if (error.response) { | |
| console.error("GitHub API error:", { | |
| status: error.response.status, | |
| message: error.response.data?.message, | |
| documentation_url: error.response.data?.documentation_url | |
| }); | |
| if (error.response.status === 404) { | |
| return res.status(404).json({ error: 'Repository not found or you do not have access.' }); | |
| } else if (error.response.status === 403) { | |
| return res.status(403).json({ error: 'Access to repository is forbidden.' }); | |
| } else { | |
| return res.status(502).json({ error: 'Error communicating with GitHub API.' }); | |
| } | |
| } | |
| console.error("Unexpected error adding repo:", error.message); | |
| res.status(500).json({ error: 'Internal server error.' }); |
| const matter = require('gray-matter'); | ||
| const { unified } = require('unified'); | ||
| const remarkParse = require('remark-parse').default; | ||
| const { remark } = require('remark'); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this change, the unified and remark-parse packages are no longer used in the codebase. Consider removing them from package.json dependencies to reduce bundle size and maintenance overhead.
Summary
Fixes issues with adding and viewing private repositories.
Changes
URL Parsing Fix
https://github.com/user/repo/)Private Repo Authentication
onAuthtogit.pull()- was missing authentication for private reposParser ESM/CJS Fix
unified().use(remarkParse)withremark().parse()require()Testing