{"record":{"id":"b3f9f824277966c4","repo":"gastownhall/beads","slug":"add-comment-q-the-commenter-reported-success-wit","errorCode":null,"errorMessage":"add comment %q: the commenter reported success without a comment","messagePattern":"add comment %q: the commenter reported success without a comment","errorType":"http","errorClass":null,"httpStatus":500,"severity":"error","filePath":"internal/httpapi/roles.go","lineNumber":259,"sourceCode":"\n// checkedCommenter is the commenter the add-comment handler is handed.\n//\n// It exists for checkedClaimer's reason exactly: handleAddComment writes\n// *result.Comment onto the wire, so a role that reported success without the row\n// would panic on a live server.\ntype checkedCommenter struct{ inner issueops.Commenter }\n\n// AddComment refuses a result that reports success without the row the response\n// body is built from.\n//\n// The generic 500, for checkedClaimer's reason. There is no wire code that fits\n// and there must not be: a 404 would say the issue does not exist when the role\n// just said it appended a comment to it, and this operation has no conflict code\n// at all. It is a broken implementation.\nfunc (c checkedCommenter) AddComment(ctx context.Context, req issueops.AddCommentRequest) (issueops.AddCommentResult, error) {\n\tresult, err := c.inner.AddComment(ctx, req)\n\tif err == nil && result.Comment == nil {\n\t\treturn issueops.AddCommentResult{}, fmt.Errorf(\"add comment %q: the commenter reported success without a comment\", req.IssueID)\n\t}\n\treturn result, err\n}\n\n// checkedReleaser is the releaser the release handler is handed.\n//\n// It exists for checkedClaimer's reason exactly: handleRelease writes\n// *result.Issue and reads its RowVersion, so a role that reported success\n// without the row would panic on a live server.\ntype checkedReleaser struct{ inner issueops.Releaser }\n\n// Release refuses a result that reports success without the row the response\n// body is built from.\n//\n// The generic 500, for checkedClaimer's reason and one of its own: there is no\n// wire code that fits and there must not be. A 409 would say the row refused\n// the release when the role said it did not, and a 404 would say the issue does\n// not exist when nothing here knows that. It is a broken implementation.","sourceCodeStart":241,"sourceCodeEnd":277,"githubUrl":"https://github.com/gastownhall/beads/blob/71377f276968b452ee607177637970a4ff888584/internal/httpapi/roles.go#L241-L277","documentation":"checkedCommenter is a wrapper around the AddComment role that enforces a contract: a nil error must always come with a non-nil Comment. When an inner implementation returns success without a comment, the wrapper rejects it as a broken implementation rather than passing a bogus success to HTTP handlers, which would otherwise produce an inconsistent 200 with no comment payload.","triggerScenarios":"Any HTTP add-comment request where the registered inner commenter returns (AddCommentResult{}, nil) — i.e., the backing store claims append success but did not return the created comment.","commonSituations":"A custom or third-party commenter implementation violating the issueops contract; a partially-migrated backend that returns an empty result on success; a bug in a new storage driver that drops the created comment from the result.","solutions":["Fix the inner commenter implementation to always return the created Comment on nil error.","Audit the storage layer used by the commenter to confirm the comment row is actually written.","Add a unit test asserting AddCommentResult.Comment != nil on success for every implementation.","If using a third-party implementation, check its issue tracker/upstream for contract-violating versions."],"exampleFix":"// before: broken inner implementation\nfunc (s *store) AddComment(ctx context.Context, req issueops.AddCommentRequest) (issueops.AddCommentResult, error) {\n    if err := s.insertComment(ctx, req); err != nil {\n        return issueops.AddCommentResult{}, err\n    }\n    return issueops.AddCommentResult{}, nil // contract violation\n}\n// after: return the created comment\nfunc (s *store) AddComment(ctx context.Context, req issueops.AddCommentRequest) (issueops.AddCommentResult, error) {\n    c, err := s.insertComment(ctx, req)\n    if err != nil {\n        return issueops.AddCommentResult{}, err\n    }\n    return issueops.AddCommentResult{Comment: c}, nil\n}","handlingStrategy":"type-guard","validationCode":"// contract test to run against any commenter implementation\nresult, err := impl.AddComment(ctx, req)\nif err == nil && result.Comment == nil {\n    t.Fatal(\"implementation violates contract: nil Comment with nil error\")\n}","typeGuard":"// narrow the result before use\nfunc validComment(r issueops.AddCommentResult, err error) bool {\n    return err == nil && r.Comment != nil\n}","tryCatchPattern":"result, err := httpAPI.AddComment(ctx, req)\nif err != nil {\n    // includes contract violations surfaced by checkedCommenter\n    http.Error(w, err.Error(), http.StatusInternalServerError)\n    return\n}","preventionTips":["Write contract tests asserting non-nil Comment on success for every commenter implementation.","Return the created entity from the storage layer, not just an ack.","Audit third-party implementations against the issueops role contract before registering them."],"tags":["contract","http-api","nil-result","implementation-bug"],"backgroundTag":"nil-result-on-success","analyzedSha":"71377f276968b452ee607177637970a4ff888584","analyzedAt":"2026-08-30T18:55:39.744Z","schemaVersion":2},"datasetVersion":"2026-08-30T23:17:21.991Z"}