Repository navigation
Conversation
8c0d156 to
e76ef40
Compare
032a598 to
45b5005
Compare
|
I seem to be unable to get this code to work.
|
8a1bebe to
0d216d6
Compare
|
Still broken? I have rebased this branch on the top of Otherwise, I really don’t want to work on this PR any more. Given the sharp anti-AI stance of SourceHut, I am moving elsewhere. Anybody, feel free, to take this PR over. |
d146faa to
beae5b8
Compare
Add a Billy-backed test storage implementation and protect mock repository state against concurrent access. Keep test infrastructure separate from the new bridge so reviewers can assess the storage change independently.
Record the API schema beside the bridge so client queries, mutations, and response types can be reviewed against the service contract without relying on an external copy.
Add authenticated HTTP transport, GraphQL request and response types, pagination queries, and ticket, event, and label mutations. Keep the protocol client separate from bridge synchronization and configuration.
Register the bridge, resolve tracker credentials, import remote tickets and events, and export local operations. Combine the mutually dependent importer and exporter so the bridge can compile as one implementation commit.
Provide mock GraphQL methods and tests for the client, configuration, debug transport, importer, and exporter. Fold compilation repairs into the tests rather than leaving later fixes to earlier test commits.
mcepl
changed the title
Document tracker URL, login, token, and bridge usage in help and reference pages. Include the design plan with the user-facing guide.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… handling Address feedback from code review on the todo.sr.ht bridge: - Require verified tracker and base URL metadata before accepting creation metadata ticket IDs on export, preventing operations on unverified bugs from modifying tickets on the configured tracker. - Track imported event operations with change-level IDs to prevent ErrMultipleMatch on repeat pulls of events containing multiple changes. - Reconcile remote edits to ticket title and body during import when reusing existing bugs. - Support explicit null body in UpdateTicketInput to allow clearing ticket descriptions on SourceHut without losing edits. - Make label export idempotent by checking remote ticket labels before adding or removing them. - Ensure credentials added without base URL metadata can be used to initialize the bridge, and tag selected credentials with base URL. - Isolate user identities by instance base URL to avoid collisions across multiple SourceHut instances. - Align ValidParams and error messages with the --project flag. - Establish chronological order for imported events across pages. - Silence repetitive warnings for unsupported assignment and mention events. - Surface server GraphQL error details on non-200 HTTP responses and clean unused variables when querying the authenticated user. - Document label color limitations and replace the obsolete roadmap in the design documentation.
| } | ||
|
|
||
| // Fetch current remote ticket to reconcile labels idempotently | ||
| currentTicket, err := client.GetTicket(ctx, bugTodoSourceHutID) |
There was a problem hiding this comment.
GetTicket, whose query requests ticket(id:) at the GraphQL root. The checked-in SourceHut schema exposes that field only under a tracker. The request therefore fails validation, so label changes cannot be pushed. The schema test does not include this query.
|
|
||
| var matchingCreds []auth.Credential | ||
| for _, cred := range creds { | ||
| if base, ok := cred.GetMetadata(auth.MetaKeyBaseURL); !ok || base == conf[confKeyBaseUrl] { |
There was a problem hiding this comment.
bridge auth add-token has login metadata but no base URL, so this filter accepts it for any configured instance. If a user has such a token for instance A alongside a configured token for instance B with the same login, pull can select A’s token and send it to B on the first tracker lookup. The exporter accepts the same token in cacheAllClient. Configuring B verifies its selected token, not the other stored tokens. How this was verified: Login-only tokens pass both instance filters, and the client attaches the selected token to requests sent to the configured base URL.
| if err := json.Unmarshal(*c.Author, &author); err != nil { | ||
| return "", err | ||
| } | ||
| return author.CanonicalName + "\x00" + c.Text, nil |
There was a problem hiding this comment.
| cleanSubject := text.CleanupOneLine(ticket.Subject) | ||
| metadata := map[string]string{metaKeyTodoSourceHutId: fmt.Sprintf("ticket:%d:%d", ticket.Id, editTime)} | ||
| if cleanSubject != "" && cleanSubject != snapshot.Title { | ||
| op, err := b.SetTitleRaw(author, editTime, cleanSubject, metadata) | ||
| if err != nil { | ||
| return nil, false, err | ||
| } | ||
| ji.out <- core.NewImportTitleEdition(b.Id(), op.Id()) | ||
| } | ||
|
|
||
| cleanBody := text.Cleanup(ticket.Body) | ||
| if len(snapshot.Comments) > 0 && cleanBody != snapshot.Comments[0].Message { | ||
| commentID, _, err := b.EditCreateCommentRaw(author, editTime, cleanBody, metadata) |
There was a problem hiding this comment.
--no-resume against the unchanged remote ticket creates an imported edit that restores the old remote value. This overwrites the local change before it can be pushed.
| out <- core.NewExportWarning(fmt.Errorf("label '%s' not found on SourceHut, cannot remove", labelName), b.Id()) | ||
| continue | ||
| } | ||
| if _, onTicket := currentLabelNames[labelName]; !onTicket { |
There was a problem hiding this comment.
Bug and the remote label is named bug, the exporter finds its ID, then decides it is absent and skips the removal. It still marks the operation exported, leaving the label on the remote ticket with no retry. The same mismatch can also cause a redundant add.
| cleanSubject := text.CleanupOneLine(ticket.Subject) | ||
| metadata := map[string]string{metaKeyTodoSourceHutId: fmt.Sprintf("ticket:%d:%d", ticket.Id, editTime)} | ||
| if cleanSubject != "" && cleanSubject != snapshot.Title { | ||
| op, err := b.SetTitleRaw(author, editTime, cleanSubject, metadata) |
There was a problem hiding this comment.
|
As a SourceHut user looking to move from GitHub, I see here:
When I flagged this code did not work I had already tried rebasing before you did so yourself and recommend I retry. That sort of hints you do not use this code day-to-day and had not tried your rebasing effort which makes me somewhat twitchy about using it. What do you recommend SourceHut users do here? Besides I suspect that community is unlikely to receive well a majority LLM written bridge :) For me, I am going to write my own bridge. |
Yes, I am leaving Sourcehut (partially because of their LLM policy, but mostly because my users just seem to irrationally hate email and I have much better feedback on web-based forge; and of course, there are other reasons as well), and yes I expect to really use daily Gitea/Forgejo so #1628 is much more relevant to me. Mostly, I wanted to make this as a kind of farewell letter to Sourcehut, and I hate to leave an unfinished business behind. Concerning the quality of coding: I am not a native Golang programmer, so I cannot really evaluate how good the best models are writing it, but with Python (and C to some extent), which are languages are used every day for my work, it seems to me that the latest models (Opus 5.5 and ChatGPT 5+ ones) are on the same level or better than what I could write by hand, much much better than they were just a year ago when I started to write this PR. What should Sourcehut users do? Well, I would hope they could take this branch, and take it further and maintain it. I believe that there is a lot of work and testing I did (completely manually, it is a way more than me blindly pressing Enter to |
This branch has not been deployed
2 participants
Adds a bridge for SourceHut's issue tracker, todo.sr.ht, using its GraphQL API. The first version was written with heavy AI assistance. I have since reviewed it, fixed what review and live testing found, and tested it against a real tracker. Reviews from people who know Go and the git-bug internals better than I do are very welcome.
Fixes: #1024
What it does
git bug bridge new --target=todosrht --url=https://todo.sr.ht/~owner/tracker --login=USERconfigures the bridge. It verifies the token by looking up the tracker.Commits
test(repository): makes the mock repository storage safe for concurrent access.bridge/todosrht: vendors the todo.sr.ht GraphQL schema (schema.graphqls). Tests validate every query and mutation against it.bridge/todosrht: adds the GraphQL client.bridge/todosrht: adds configuration, import, export and bridge registration.test(bridge/todosrht): adds mock-based tests, schema validation, and HTTP tests for request paths and redirects.docs: adds the usage guide, command help, man pages and the design notes.Security
GIT_BUG_DEBUG=1dumps requests and responses with the token redacted, and truncates response bodies.Testing
go test ./...passes.hut. Repeated pulls and pushes made no further changes, andhutconfirmed the ticket state.Known limitations
todo.sr.ht/PROFILE:RO todo.sr.ht/EVENTS:RO todo.sr.ht/TRACKERS:RW todo.sr.ht/TICKETS:RW) reaches every tracker the account can access.sinceand scans all local bugs. Metadata prevents duplicate exports, but push time grows with the number of bugs.~owner/name) are built by rewriting theme { ... }query templates. Schema tests cover the result, but separate templates would be clearer.TrackerExistsnow only wrapsGetTracker, andclient_test.gotests only the mock client. Both could be removed.The PR is not safe to merge while label exports fail and credential selection can send a token to the wrong SourceHut instance.
Findings
Summary
This PR adds a SourceHut todo.sr.ht bridge with GraphQL configuration, ticket and event import, operation export, tests, and usage documentation. It also adds concurrent-safe mock storage.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR S[SourceHut tracker] -->|tickets and events| I[Importer] I --> G[git-bug operations] G --> E[Exporter] E -->|GraphQL mutations| S K[Stored credentials] --> I K --> EReviews (2) · Last reviewed commit: "test(bridge/todosrht): commit operations..."